From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stephen Berman Newsgroups: gmane.emacs.bugs Subject: bug#52856: 29.0.50; Problematic handling of webkit xwidget bookmarks Date: Thu, 30 Dec 2021 11:52:20 +0100 Message-ID: <877dbm1i1n.fsf@gmx.net> References: <875yr8wn9e.fsf@gmx.net> <87ilv7ub1o.fsf@yahoo.com> <87sfubfzwt.fsf@gmx.net> <874k6rtzy7.fsf@yahoo.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="35381"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 52856@debbugs.gnu.org To: Po Lu Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Dec 30 11:53:15 2021 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1n2t3a-00094W-W2 for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 30 Dec 2021 11:53:15 +0100 Original-Received: from localhost ([::1]:50946 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n2t3Z-0004z8-KH for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 30 Dec 2021 05:53:13 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:33096) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n2t3O-0004z0-CA for bug-gnu-emacs@gnu.org; Thu, 30 Dec 2021 05:53:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:39948) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n2t3O-0000UP-34 for bug-gnu-emacs@gnu.org; Thu, 30 Dec 2021 05:53:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1n2t3O-0002HA-3t for bug-gnu-emacs@gnu.org; Thu, 30 Dec 2021 05:53:02 -0500 X-Loop: help-debbugs@gnu.org In-Reply-To: <875yr8wn9e.fsf@gmx.net> Resent-From: Stephen Berman Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 30 Dec 2021 10:53:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 52856 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 52856-submit@debbugs.gnu.org id=B52856.16408615528697 (code B ref 52856); Thu, 30 Dec 2021 10:53:02 +0000 Original-Received: (at 52856) by debbugs.gnu.org; 30 Dec 2021 10:52:32 +0000 Original-Received: from localhost ([127.0.0.1]:51494 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1n2t2u-0002GC-1O for submit@debbugs.gnu.org; Thu, 30 Dec 2021 05:52:32 -0500 Original-Received: from mout.gmx.net ([212.227.17.21]:59219) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1n2t2s-0002Fx-4t for 52856@debbugs.gnu.org; Thu, 30 Dec 2021 05:52:31 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1640861542; bh=BKmV+rqSNpSLnzX6nD9GcZTqsjRZuw4jlhjp5bOOMgE=; h=X-UI-Sender-Class:From:To:Cc:Subject:References:Date; b=LGD8qMgQHq7Fr3tE5KJEPVxzt9XK9GagpDgpHLwmlVCjJBlERzjXOzUntQ3tZHiez PTVkYYhYnSQD9tGWDxyGLBEn99MeCzZQcxdwm+vJY5FKezfwcxGY8V20n85TUxNs+h K9e1NCvEGKOqnpaBqci3Nktj8ywTyPujdJjDNzgw= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Original-Received: from strobelfs ([89.246.38.232]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1N0oG5-1mF3gJ0BRV-00winq; Thu, 30 Dec 2021 11:52:22 +0100 X-Provags-ID: V03:K1:UurrdqrX/zhOhWpeydpWllKDGWnu27PyYJLnCU1fZs/ttnP8Rdm 8ACBopw0wrGsq86j7RKEwRfUGKSnQNVvQ16YXU0wRMpnpPeX5hWZXB6d1piEk3WnYDvtpdH ksiCtgQlimHm9mzNCIcTaoMzh5AEfH3LvnKtgBY0yCVzr8I7lfx+vXZA/vXDv//ymupkOth emYcCtbFp8InpAJJyW6jQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:D2X66cZpTrU=:Uw+ehocFwX4Ha+A9kxUtGo 5z468/sGLPhcO49oLbrJY5Joshpwg4mKPx0Uhkp+prMooiQAPNY+TRsJ+5Nyzu0Z+eEXUOEe1 nYM3nK/9n8JqyYVdGQE24KnwQ0BRmF1Sf4nK6ZanCt7vjBirFabGTwnZnde0wj81XP1lA6WOi qBhYxe3LqwgmsvAOi5tKB4he10vMwD9jLHCZiDxSC4gQXvaCxbGUE8ha2kOxiknwaBT2+qcUD QZkFk7hzVO4ZGMNFy0oFO9McHT1HUCL3jOsTycU9mQPOtqet24HDrtVPLEwd4OjkCyJ1nXZzf zZKQAAXbwFW259RThVqyk/KkkCMVElMcehBkOuuLTPuLwGUHS9yraVzpcX0ONLjiZfS4M6Yhd aDlnREkvdp/PcbTpa8UOszWTy8Alf1gmcnTIWtRvrajolgd57BPcoHIrQWrUure4mwMhjKERu TQ8/FnD+eonIUCQdRex/DmbIiqwy7iv5qccQmr2GyT7MJHArSLFzS0btAw/n7DCJzWUsBKNP8 XIy7fEjihwMBpMlB3B3AXI9lH5dZl9iBaZndE5YRUIj7Rvh+n8MLA94/zci4kQUG+fhZdIz5b UqYEI1Z3mA4w9hGxxmca80GsmtRp2L92EvkNzt73cxskOl8OCL9xPx5w01LpKStXKzfCzcygb 0hR7r4VlrWCQlA4wVAP93YvlC1Xb9MKOLmGI/fVFgqINyR62za7PYeyvZA2RwPblTKIXsr9j0 pBXvWFxpYQCD4bgKAg/ozVK9Kz+BcG18szeIhhtOIxZhwOIYj8H+9frhIYSX/VxGhF5brJNH X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:223422 Archived-At: --=-=-= Content-Type: text/plain On Wed, 29 Dec 2021 19:25:52 +0800 Po Lu wrote: > Stephen Berman writes: > >> I did, but that just ensures that jumping to the bookmarked xwidget >> creates a new xwidget session -- it does not prevent the xwidget from >> being displayed both in the other window/frame and in the originally >> selected window. My patch ensures that the bookmarked xwidget is >> displayed only in the other window/frame, which is consistent with the >> behavior of `bookmark-jump-other-window' and `bookmark-jump-other-frame' >> with other types of bookmarks (e.g. to PDFs in the pdf-tools package), >> regardless of whether a new xwidget session is created. > > Okay, thanks -- some comments below: > >> + (with-temp-buffer >> + (xwidget-webkit-new-session url) >> + (current-buffer)) > > Why with-temp-buffer? Oh, that's a faux pas, the residue of an earlier version; `progn' would have sufficed, but the new version below does it differently anyway. > xwidget-webkit-new-session creates a new buffer and switches to it, > which is not appropriate if you want to obtain a buffer containing the > new xwidget. I suggest that you write a new function that creates the > buffer, displays the xwidget, and returns the buffer, preferably also > updating xwidget-webkit-new-session to use it as well. > >> + (switch-to-buffer curbuf) >> + (set-buffer xwbuf))) > > See above. Thanks for the suggestion, see the patch below. Since the previous bookmark handler was the only user of `xwidget-webkit-browse-url''s `new-session' parameter, I removed that, and took the opportunity to make some other cleanups to that function, see the commit message. While testing the refactored `xwidget-webkit-new-session' I found that the xwidget-webkit-clone-* commands were broken because `xwidget-webkit-current-url' returned the message instead of the URL, so I fixed that. And I removed from the doc string of `xwidget-webkit-bookmark-jump-new-session' the suggestion to customize `xwidget-webkit-last-session-buffer', since, for one thing, it's not a defcustom, but more importantly, changing it has no effect, since it gets set by `xwidget-webkit--create-new-session-buffer' (previously by `xwidget-webkit-new-session'), overriding any user change to its value. 2021-12-30 Stephen Berman Fix handling of webkit xwidget bookmarks Make jumping to a bookmarked webkit xwidget in another window or another frame show the xwidget only in that window or frame. Adjust xwidget.el code accordingly. Make xwidget-webkit-clone-* commands work. (Bug#52856) * lisp/xwidget.el (xwidget-webkit-browse-url): Remove `new-session' parameter, since the bookmark handler, which was its only user, no longer calls this function. Remove superfluous `browse-url' require from interactive spec and discard the unused flag returned by `browse-url-interactive-arg'. Incorporate the code of the now removed `xwidget-webkit-goto-url', since this function was its only caller. (xwidget-webkit-goto-url): Remove. (xwidget-webkit-bookmark-jump-new-session): Adjust doc string, rephrasing and removing a customization suggestion that cannot take effect. (xwidget-webkit-bookmark-jump-handler): New autoloaded function. (xwidget-webkit-bookmark-make-record): Use it. (xwidget-webkit--create-new-session-buffer): New function extracted from `xwidget-webkit-new-session'. (xwidget-webkit-new-session): Use it. (xwidget-webkit-current-url): Return the URL instead of the message displaying it, since the xwidget-webkit-clone-* commands need it. --=-=-= Content-Type: text/x-patch Content-Disposition: inline Content-Description: xwidget.el patch Content-Transfer-Encoding: quoted-printable diff --git a/lisp/xwidget.el b/lisp/xwidget.el index ce9839ebd3..42b5e08055 100644 =2D-- a/lisp/xwidget.el +++ b/lisp/xwidget.el @@ -116,22 +116,21 @@ xwidget-webkit-cookie-file :version "29.1") ;;;###autoload -(defun xwidget-webkit-browse-url (url &optional new-session) - "Ask xwidget-webkit to browse URL. -NEW-SESSION specifies whether to create a new xwidget-webkit session. -Interactively, URL defaults to the string looking like a url around point= ." - (interactive (progn - (require 'browse-url) - (browse-url-interactive-arg "xwidget-webkit URL: "))) +(defun xwidget-webkit-browse-url (url) + "Prompt for a URL and display it in a webkit xwidget. +The URL defaults to the string looking like a url around point." + (interactive (list (car (browse-url-interactive-arg "xwidget-webkit URL= : ")))) (or (featurep 'xwidget-internal) (user-error "Your Emacs was not compiled with xwidgets support")) (when (stringp url) ;; If it's a "naked url", just try adding https: to it. (unless (string-match "\\`[A-Za-z]+:" url) (setq url (concat "https://" url))) - (if new-session - (xwidget-webkit-new-session url) - (xwidget-webkit-goto-url url)))) + (if (xwidget-webkit-current-session) + (progn + (xwidget-webkit-goto-uri (xwidget-webkit-current-session) url) + (switch-to-buffer (xwidget-buffer (xwidget-webkit-current-sessi= on)))) + (xwidget-webkit-new-session url)))) (defun xwidget-webkit-clone-and-split-below () "Clone current URL into a new widget place in new window below. @@ -531,24 +530,31 @@ xwidget-webkit-save-as-file ;;; Bookmarks integration (defcustom xwidget-webkit-bookmark-jump-new-session nil - "Control bookmark jump to use new session or not. -If non-nil, use a new xwidget webkit session after bookmark jump. -Otherwise, it will use `xwidget-webkit-last-session'. -When you set this variable to nil, consider further customization with -`xwidget-webkit-last-session-buffer'." + "Whether to jump to a bookmarked URL in a new xwidget webkit session. +If non-nil, create a new xwidget webkit session, otherwise use +the value of `xwidget-webkit-last-session'." :version "28.1" :type 'boolean) (defun xwidget-webkit-bookmark-make-record () - "Create bookmark record in webkit xwidget. -See `xwidget-webkit-bookmark-jump-new-session' for whether this -should create a new session or not." + "Create a bookmark record for a webkit xwidget." (nconc (bookmark-make-record-default t t) `((page . ,(xwidget-webkit-uri (xwidget-webkit-current-session))= ) - (handler . (lambda (bmk) - (xwidget-webkit-browse-url - (bookmark-prop-get bmk 'page) - xwidget-webkit-bookmark-jump-new-session)))))) + (handler . xwidget-webkit-bookmark-jump-handler)))) + +;;;###autoload +(defun xwidget-webkit-bookmark-jump-handler (bmk) + "Function for jumping to the webkit xwidget bookmarked by BMK. +If `xwidget-webkit-bookmark-jump-new-session' is non-nil, create +a new xwidget-webkit session, otherwise use an existing session." + (let* ((url (bookmark-prop-get bmk 'page)) + (xwbuf (if (or xwidget-webkit-bookmark-jump-new-session + (not (xwidget-webkit-current-session))) + (xwidget-webkit--create-new-session-buffer url) + (xwidget-buffer (xwidget-webkit-current-session))))) + (with-current-buffer xwbuf + (xwidget-webkit-goto-uri (xwidget-webkit-current-session) url)) + (set-buffer xwbuf))) ;;; xwidget webkit session @@ -796,37 +802,44 @@ xwidget-webkit-adjust-size-in-frame (add-to-list 'window-size-change-functions 'xwidget-webkit-adjust-size-in-frame)) -(defun xwidget-webkit-new-session (url &optional callback) - "Create a new webkit session buffer with URL." +(defun xwidget-webkit--create-new-session-buffer (url &optional callback) + "Create a new webkit session buffer to display URL in an xwidget. +Optional function CALLBACK specifies the callback for webkit xwidgets; +see `xwidget-webkit-callback'." (let* ((bufname - ;; Generate a temp-name based on current buffer name. it - ;; will be renamed by `xwidget-webkit-callback' in the - ;; future. This approach can limit flicker of buffer-name in - ;; mode-line. + ;; Generate a temp-name based on current buffer name. The + ;; buffer will subsequently be renamed by + ;; `xwidget-webkit-callback'. This approach can avoid + ;; flicker of buffer-name in mode-line. (generate-new-buffer-name (buffer-name))) (callback (or callback #'xwidget-webkit-callback)) (current-session (xwidget-webkit-current-session)) xw) - (setq xwidget-webkit-last-session-buffer (switch-to-buffer - (get-buffer-create bufname)= )) + (setq xwidget-webkit-last-session-buffer (get-buffer-create bufname)) ;; The xwidget id is stored in a text property, so we need to have ;; at least character in this buffer. ;; Insert invisible url, good default for next `g' to browse url. - (let ((start (point))) - (insert url) - (put-text-property start (+ start (length url)) 'invisible t) - (setq xw (xwidget-insert - start 'webkit bufname - (xwidget-window-inside-pixel-width (selected-window)) - (xwidget-window-inside-pixel-height (selected-window)) - nil current-session))) - (when xwidget-webkit-cookie-file - (xwidget-webkit-set-cookie-storage-file - xw (expand-file-name xwidget-webkit-cookie-file))) - (xwidget-put xw 'callback callback) - (xwidget-put xw 'display-callback #'xwidget-webkit-display-callback) - (xwidget-webkit-mode) - (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url))) + (with-current-buffer xwidget-webkit-last-session-buffer + (let ((start (point))) + (insert url) + (put-text-property start (+ start (length url)) 'invisible t) + (setq xw (xwidget-insert + start 'webkit bufname + (xwidget-window-inside-pixel-width (selected-window)) + (xwidget-window-inside-pixel-height (selected-window)) + nil current-session))) + (when xwidget-webkit-cookie-file + (xwidget-webkit-set-cookie-storage-file + xw (expand-file-name xwidget-webkit-cookie-file))) + (xwidget-put xw 'callback callback) + (xwidget-put xw 'display-callback #'xwidget-webkit-display-callback= ) + (xwidget-webkit-mode)) + xwidget-webkit-last-session-buffer)) + +(defun xwidget-webkit-new-session (url) + "Display URL in a new webkit xwidget." + (switch-to-buffer (xwidget-webkit--create-new-session-buffer url)) + (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url)) (defun xwidget-webkit-import-widget (xwidget) "Create a new webkit session buffer from XWIDGET, an existing xwidget. @@ -868,14 +881,6 @@ xwidget-webkit-display-callback (define-key special-event-map [xwidget-display-event] 'xwidget-webkit-dis= play-event) -(defun xwidget-webkit-goto-url (url) - "Goto URL with xwidget webkit." - (if (xwidget-webkit-current-session) - (progn - (xwidget-webkit-goto-uri (xwidget-webkit-current-session) url) - (switch-to-buffer (xwidget-buffer (xwidget-webkit-current-session= )))) - (xwidget-webkit-new-session url))) - (defun xwidget-webkit-back () "Go back to previous URL in xwidget webkit buffer." (interactive nil xwidget-webkit-mode) @@ -892,10 +897,12 @@ xwidget-webkit-reload (xwidget-webkit-goto-history (xwidget-webkit-current-session) 0)) (defun xwidget-webkit-current-url () - "Display the current xwidget webkit URL and place it on the `kill-ring'= ." + "Return current xwidget webkit URL and display it in a message. +Also place it on the `kill-ring'." (interactive nil xwidget-webkit-mode) (let ((url (xwidget-webkit-uri (xwidget-webkit-current-session)))) - (message "URL: %s" (kill-new (or url ""))))) + (message "URL: %s" (kill-new (or url ""))) + url)) (defun xwidget-webkit-browse-history () "Display a buffer containing the history of page loads." --=-=-=--