all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Your last change to browse-url is bogus.
@ 2007-09-12  9:13 Michaël Cadilhac
  2007-09-12 10:20 ` Johannes Weiner
  0 siblings, 1 reply; 18+ messages in thread
From: Michaël Cadilhac @ 2007-09-12  9:13 UTC (permalink / raw)
  To: emacs-devel; +Cc: Johannes Weiner


[-- Attachment #1.1.1: Type: text/plain, Size: 1113 bytes --]

This change:

+2007-09-07  Johannes Weiner  <hannes@saeurebad.de>
+
+	* 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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: browse-url.patch --]
[-- Type: text/x-patch, Size: 4020 bytes --]

--- 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()")))

[-- Attachment #1.1.3: Type: text/plain, Size: 329 bytes --]



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

[-- Attachment #1.2: Type: application/pgp-signature, Size: 188 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2007-09-17 11:52 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-12  9:13 Your last change to browse-url is bogus Michaël Cadilhac
2007-09-12 10:20 ` Johannes Weiner
2007-09-12 10:38   ` Michaël Cadilhac
2007-09-12 10:49     ` Johannes Weiner
2007-09-12 11:09       ` Michaël Cadilhac
2007-09-12 11:25         ` Johannes Weiner
2007-09-12 11:46           ` Michaël Cadilhac
2007-09-12 12:54     ` YAMAMOTO Mitsuharu
2007-09-12 13:13       ` Michaël Cadilhac
2007-09-12 14:18         ` YAMAMOTO Mitsuharu
2007-09-12 22:50           ` Michaël Cadilhac
2007-09-17  4:40             ` YAMAMOTO Mitsuharu
2007-09-17 11:52               ` Michaël Cadilhac
2007-09-12 23:29         ` Stephen J. Turnbull
2007-09-13  2:30           ` Stefan Monnier
2007-09-13 18:15             ` Stephen J. Turnbull
2007-09-13 21:44               ` Stefan Monnier
2007-09-13  2:39           ` Davis Herring

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.