Re: [PATCH v2] org-protocol: Add void() to bookmarklets

[Max Nikulin <manikulin@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

On 24/12/2024 21:23, Ihor Radchenko wrote:
> Max Nikulin writes:
>>>
>>> javascript:location.href='org-protocol://store-link?'+new URLSearchParams({url:location.href, title:document.title}); void(0);
>>
>> If you think that ";void(0)" variant is less prone to errors then I may
>> update my patch. I have no idea if balanced void(...) parenthesis around
>> the whole expressions would cause more user mistakes. Otherwise both
>> variants are equivalent.
> 
> Then, no need to change your variant.

I changed the patch to trailing ";void(0)" and I have found a typo in 
first variant of the patch.

>>>> Should ORG-NEWS be updated as well?

I have realized that an example in ORG-NEWS is incomplete and so it does 
not require update.

>> I consider the change as a minor fix that does not deserves loud
>> announcement, especially taking into account that browsers discourages
>> protocol handlers launched on behalf of web pages nowadays.
[...]
> In this case, it is a good idea for users of the previously suggested
> bookmarklet to update to the new version. This way, they will be less
> likely to meet breakage one day when they decide to upgrade Firefox.

Users do not decide when they update a major browser. Security fixes 
arrive at least once a month or even more often. This particular change 
has been backported to the ESR channel, so it is too late to notify 
users through ORG-NEWS.

I have had a look into Firefox bug tracker. Behavior in respect to 
external protocol handlers has changed during a couple of years. Some of 
latest issues are not public.

[-- Attachment #2: v2-0001-org-protocol-Add-void-0-to-bookmarklets.patch --]
[-- Type: text/x-patch, Size: 6404 bytes --]

From 6e5eb4c0aedd2ab90df6f6772ded355ccb9195a4 Mon Sep 17 00:00:00 2001
From: Max Nikulin <manikulin@gmail.com>
Date: Mon, 16 Dec 2024 21:46:49 +0700
Subject: [PATCH v2] org-protocol: Add void(0) to bookmarklets

* doc/org-manual.org (The store-link protocol):
(The capture protocol, The open-source protocol):
* lisp/org-protocol.el (org-protocol-store-link):
(org-protocol-capture, org-protocol-open-source): Update javascript:
bookmarklets to discard assigned value.

Firefox-133 and Firefox-128.5 ESR does not discard value assigned to
window.location anymore and use it as document content to render instead.

Reported by: Rehan Deen.  [BUG] Org-protocol bookmarklets in Firefox
behaving badly after recent upgrade.  Thu, 12 Dec 2024 13:42:34 +0530.
Link: <https://list.orgmode.org/87pllx7219.fsf@gmail.com>
---
 doc/org-manual.org   |  9 +++++----
 lisp/org-protocol.el | 21 +++++++++++----------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/doc/org-manual.org b/doc/org-manual.org
index 1b3c33f96..2d5fd57b3 100644
--- a/doc/org-manual.org
+++ b/doc/org-manual.org
@@ -21327,14 +21327,14 @@ *** The ~store-link~ protocol
 
 #+begin_example
 javascript:location.href='org-protocol://store-link?' +
-      new URLSearchParams({url:location.href, title:document.title});
+      new URLSearchParams({url:location.href, title:document.title});void(0);
 #+end_example
 
 Title is an optional parameter.  Another expression was recommended earlier:
 
 #+begin_example
 javascript:location.href='org-protocol://store-link?url='+
-      encodeURIComponent(location.href);
+      encodeURIComponent(location.href);void(0);
 #+end_example
 
 The latter form is compatible with older Org versions from 9.0 to 9.4.
@@ -21359,6 +21359,7 @@ *** The ~capture~ protocol
       new URLSearchParams({
             template: 'x', url: window.location.href,
             title: document.title, body: window.getSelection()});
+      void(0);
 #+end_example
 
 You might have seen another expression:
@@ -21367,7 +21368,7 @@ *** The ~capture~ protocol
 javascript:location.href='org-protocol://capture?template=x'+
       '&url='+encodeURIComponent(window.location.href)+
       '&title='+encodeURIComponent(document.title)+
-      '&body='+encodeURIComponent(window.getSelection());
+      '&body='+encodeURIComponent(window.getSelection());void(0);
 #+end_example
 
 It is a bit more cluttered than the former one, but it is compatible
@@ -21400,7 +21401,7 @@ *** The ~open-source~ protocol
 
 #+begin_example
 javascript:location.href='org-protocol://open-source?&url='+
-      encodeURIComponent(location.href)
+      encodeURIComponent(location.href);void(0)
 #+end_example
 
 #+vindex: org-protocol-project-alist
diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index 85c9a0cc5..1d3bb7e71 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -101,15 +101,15 @@ ;;; Commentary:
 ;;           new URLSearchParams({
 ;;                 url: location.href,
 ;;                 title: document.title,
-;;                 body: window.getSelection()})
+;;                 body: window.getSelection()});void(0)
 ;;
 ;; Alternatively use the following expression that encodes space as \"%20\"
 ;; instead of \"+\", so it is compatible with Org versions from 9.0 to 9.4:
 ;;
-;;     location.href='org-protocol://sub-protocol?url='+
+;;     javascript:location.href='org-protocol://sub-protocol?url='+
 ;;           encodeURIComponent(location.href)+'&title='+
 ;;           encodeURIComponent(document.title)+'&body='+
-;;           encodeURIComponent(window.getSelection())
+;;           encodeURIComponent(window.getSelection());void(0)
 ;;
 ;; The handler for the sub-protocol \"capture\" detects an optional template
 ;; char that, if present, triggers the use of a special template.
@@ -437,14 +437,14 @@ (defun org-protocol-store-link (fname)
 The location for a browser's bookmark may look like this:
 
   javascript:location.href = \\='org-protocol://store-link?\\=' +
-       new URLSearchParams({url:location.href, title:document.title});
+       new URLSearchParams({url:location.href, title:document.title});void(0);
 
 or to keep compatibility with Org versions from 9.0 to 9.4 it may be:
 
   javascript:location.href = \\
       \\='org-protocol://store-link?url=\\=' + \\
       encodeURIComponent(location.href) + \\='&title=\\=' + \\
-      encodeURIComponent(document.title);
+      encodeURIComponent(document.title);void(0);
 
 Don't use `escape()'!  Use `encodeURIComponent()' instead.  The
 title of the page could contain slashes and the location
@@ -482,20 +482,21 @@ (defun org-protocol-capture (info)
         new URLSearchParams({
               url: location.href,
               title: document.title,
-              body: window.getSelection()})
+              body: window.getSelection()});void(0)
 
 or to keep compatibility with Org versions from 9.0 to 9.4:
 
   javascript:location.href = \\='org-protocol://capture?url=\\='+ \\
         encodeURIComponent(location.href) + \\='&title=\\=' + \\
         encodeURIComponent(document.title) + \\='&body=\\=' + \\
-        encodeURIComponent(window.getSelection())
+        encodeURIComponent(window.getSelection());void(0)
 
 By default, it uses the character `org-protocol-default-template-key',
 which should be associated with a template in `org-capture-templates'.
 You may specify the template with a template= query parameter, like this:
 
-  javascript:location.href = \\='org-protocol://capture?template=b\\='+ ...
+  javascript:location.href
+        = \\='org-protocol://capture?template=b\\='+ ...;void(0)
 
 Now template ?b will be used."
   (let* ((parts
@@ -556,13 +557,13 @@ (defun org-protocol-open-source (fname)
 The location for a browser's bookmark should look like this:
 
   javascript:location.href = \\='org-protocol://open-source?\\=' +
-        new URLSearchParams({url: location.href})
+        new URLSearchParams({url: location.href});void(0)
 
 or if you prefer to keep compatibility with older Org versions (9.0 to 9.4),
 consider the following expression:
 
   javascript:location.href = \\='org-protocol://open-source?url=\\=' + \\
-        encodeURIComponent(location.href)"
+        encodeURIComponent(location.href);void(0)"
   ;; As we enter this function for a match on our protocol, the return value
   ;; defaults to nil.
   (let (;; (result nil)
-- 
2.39.5