emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Your last change to browse-url is bogus.


From: Michaël Cadilhac
Subject: Your last change to browse-url is bogus.
Date: Wed, 12 Sep 2007 11:13:42 +0200
User-agent: Gnus/5.110007 (No Gnus v0.7) Emacs/23.0.50 (gnu/linux)

This change:

+2007-09-07  Johannes Weiner  <address@hidden>
+
+       * net/browse-url.el (browse-url-browser-function): Add elinks.
+       (browse-url-elinks-wrapper): New option.
+       (browse-url-encode-url, browse-url-elinks)
+       (browse-url-elinks-sentinel): New functions.
+       (browse-url-file-url, browse-url-netscape, browse-url-mozilla)
+       (browse-url-firefox, browse-url-galeon, browse-url-epiphany): Use
+       new function browse-url-encode-url.
+

(the addition of browse-url-encode-url) creates some bugs :

1. This:

    (while (string-match "%" encoded-url)
      (setq encoded-url (replace-match "%25" t t encoded-url)))

is an infinite loop.

2. The URL 

"http://www.benzedrine.cx/lookandsay?SAME%20AS%201069";

for example, is translated to

"http://www.benzedrine.cx/lookandsay%3fSAME%2520AS%25201069";

which is utterly bogus, because the original %20 are no longer replaced
by ` ' by the server and the `?' will not be parsed.

I'd suggest to revert this change or, if it's for the sake of code
factoring, (what was the first purpose, by the way?) use something like
this:

--- browse-url.el       12 Sep 2007 10:49:04 +0200      1.61
+++ browse-url.el       12 Sep 2007 11:09:27 +0200      
@@ -619,12 +619,16 @@
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;; URL encoding
 
-(defun browse-url-encode-url (url)
-  "Encode all `confusing' characters in URL."
-  (let ((encoded-url (copy-sequence url)))
-    (while (string-match "%" encoded-url)
-      (setq encoded-url (replace-match "%25" t t encoded-url)))
-    (while (string-match "[*\"()',=;? ]" encoded-url)
+(defun browse-url-encode-url (url chars &optional encode-percent)
+  "Encode all `confusing' characters denoted by the regexp CHARS in URL.
+If ENCODE-PERCENT is non-nil, consider `%' as a confusing char."
+  (let ((s 0)
+       (encoded-url (copy-sequence url)))
+    (when encode-percent
+      (while (setq s (string-match "%" encoded-url s))
+       (setq encoded-url (replace-match "%25" t t encoded-url)
+             s (1+ s))))
+    (while (string-match chars encoded-url)
       (setq encoded-url
            (replace-match (format "%%%x"
                                   (string-to-char (match-string 0 
encoded-url)))
@@ -703,7 +707,7 @@
                     (or file-name-coding-system
                         default-file-name-coding-system))))
     (if coding (setq file (encode-coding-string file coding))))
-  (setq file (browse-url-encode-url file))
+  (setq file (browse-url-encode-url file "[*\"()',=;? ]" 'encode-percent))
   (dolist (map browse-url-filename-alist)
     (when (and map (string-match (car map) file))
       (setq file (replace-match (cdr map) t nil file))))
@@ -909,7 +913,7 @@
 When called non-interactively, optional second argument NEW-WINDOW is
 used instead of `browse-url-new-window-flag'."
   (interactive (browse-url-interactive-arg "URL: "))
-  (setq url (browse-url-encode-url url))
+  (setq url (browse-url-encode-url url "[,)$]"))
   (let* ((process-environment (browse-url-process-environment))
         (process
          (apply 'start-process
@@ -979,7 +983,7 @@
 When called non-interactively, optional second argument NEW-WINDOW is
 used instead of `browse-url-new-window-flag'."
   (interactive (browse-url-interactive-arg "URL: "))
-  (setq url (browse-url-encode-url url))
+  (setq url (browse-url-encode-url url "[,)$]"))
   (let* ((process-environment (browse-url-process-environment))
          (process
          (apply 'start-process
@@ -1037,7 +1041,7 @@
 are ignored as well.  Firefox on Windows will always open the requested
 URL in a new window."
   (interactive (browse-url-interactive-arg "URL: "))
-  (setq url (browse-url-encode-url url))
+  (setq url (browse-url-encode-url url "[,)$]"))
   (let* ((process-environment (browse-url-process-environment))
         (process
          (apply 'start-process
@@ -1089,7 +1093,7 @@
 When called non-interactively, optional second argument NEW-WINDOW is
 used instead of `browse-url-new-window-flag'."
   (interactive (browse-url-interactive-arg "URL: "))
-  (setq url (browse-url-encode-url url))
+  (setq url (browse-url-encode-url url "[,)$]"))
   (let* ((process-environment (browse-url-process-environment))
          (process (apply 'start-process
                         (concat "galeon " url)
@@ -1134,7 +1138,7 @@
 When called non-interactively, optional second argument NEW-WINDOW is
 used instead of `browse-url-new-window-flag'."
   (interactive (browse-url-interactive-arg "URL: "))
-  (setq url (browse-url-encode-url url))
+  (setq url (browse-url-encode-url url "[,)$]"))
   (let* ((process-environment (browse-url-process-environment))
          (process (apply 'start-process
                         (concat "epiphany " url)
@@ -1520,7 +1524,7 @@
 The Elinks command will be prepended by the program+arguments
 from `elinks-browse-url-wrapper'."
   (interactive (browse-url-interactive-arg "URL: "))
-  (setq url (browse-url-encode-url url))
+  (setq url (browse-url-encode-url url "[,)$]"))
   (let ((process-environment (browse-url-process-environment))
        (elinks-ping-process (start-process "elinks-ping" nil
                                             "elinks" "-remote" "ping()")))

-- 
 |   Michaël `Micha' Cadilhac       |  And please suggest to him that        |
 |   http://michael.cadilhac.name   |    he not refer to Microsoft Windows   |
 |   JID/MSN:                       |                  as "win".             |
 `----  address@hidden  |          -- RMS                   -  --'

Attachment: pgpEgIYQyMuN9.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]