bug#58118: 29.0.50; Follow #target links in eww without re-rendering page

bug#58118: 29.0.50; Follow #target links in eww without re-rendering page

[Visuwesh]

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

Severity: wishlist

eww-follow-link currently follows #target links in the same URL by
re-rendering the page.  Whilst this is fine for small HTML files, the
1.9M HTML file that I'm currently visiting takes a couple seconds to
render.
I see that eww-display-html follows #target links by a text property
search, why don't we do the same in eww-follow-link as well?  I have one
question though: what do we about non-existent #targets?  Chrome and
Firefox seem to not scroll when I add a non-existent #target to the
current URL and say RET, and it seems to specially handle #top to mean
to go to the top even if the HTML has no hits for "#top".  Should we
handle the special #top target?
By non-existent targets, I mean to say that

    (goto-char (point-min))
    (text-property-search-forward 'shr-target-id "invalid" #'member)

returns nil for a non-existent "#invalid" target.

Tentative patch attached,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: eww-target.patch --]
[-- Type: text/x-diff, Size: 1594 bytes --]

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 61b1b75068..6ac198099a 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -315,7 +315,8 @@ eww-accept-content-types
 
 (defvar-keymap eww-link-keymap
   :parent shr-map
-  "RET" #'eww-follow-link)
+  "RET" #'eww-follow-link
+  "<mouse-2>" #'eww-follow-link)
 
 (defvar-keymap eww-image-link-keymap
   :parent shr-map
@@ -1901,7 +1902,8 @@ eww-follow-link
    eww-mode)
   (mouse-set-point mouse-event)
   (let* ((orig-url (get-text-property (point) 'shr-url))
-         (url (eww--transform-url orig-url)))
+         (url (eww--transform-url orig-url))
+         target)
     (cond
      ((not url)
       (message "No link under point"))
@@ -1913,12 +1915,17 @@ eww-follow-link
       (funcall browse-url-secondary-browser-function url)
       (shr--blink-link))
      ;; This is a #target url in the same page as the current one.
-     ((and (url-target (url-generic-parse-url url))
+     ((and (setq target (url-target (url-generic-parse-url url)))
 	   (eww-same-page-p url (plist-get eww-data :url)))
-      (let ((dom (plist-get eww-data :dom)))
+      (let ((dom (plist-get eww-data :dom))
+            match)
 	(eww-save-history)
 	(plist-put eww-data :url url)
-	(eww-display-html 'utf-8 url dom nil (current-buffer))))
+        (goto-char (point-min))
+        (setq match (text-property-search-forward 'shr-target-id target #'member))
+        (if match
+            (goto-char (prop-match-beginning match))
+	  (eww-display-html 'utf-8 url dom nil (current-buffer)))))
      (t
       (eww-browse-url orig-url external)))))
 

[-- Attachment #3: Type: text/plain, Size: 6142 bytes --]


In GNU Emacs 29.0.50 (build 31, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw scroll bars) of 2022-09-27 built on astatine
Repository revision: 51ec68b3182914c6c95d6ddcd88ae75239ec2904
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Debian GNU/Linux bookworm/sid

Configured using:
 'configure --with-x-toolkit=lucid --without-xaw3d'
Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER
PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11
XDBE XIM XINPUT2 XPM LUCID ZLIB
Important settings:
  value of $LC_MONETARY: ta_IN.UTF-8
  value of $LC_NUMERIC: ta_IN.UTF-8
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: ELisp/l

Minor modes in effect:
  shell-dirtrack-mode: t
  recentf-mode: t
  bug-reference-prog-mode: t
  paredit-mode: t
  eros-mode: t
  flymake-mode: t
  pdf-occur-global-minor-mode: t
  minibuffer-depth-indicate-mode: t
  repeat-mode: t
  display-time-mode: t
  display-battery-mode: t
  winner-mode: t
  vz/acme-tag-minor-mode: t
  delete-selection-mode: t
  xterm-mouse-mode: t
  straight-use-package-mode: t
  straight-package-neutering-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  use-hard-newlines: t
  tab-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  undelete-frame-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
None found.

Features:
(shadow sort flyspell ispell ecomplete mail-extr emacsbug message
yank-media rfc822 mml mml-sec epa epg rfc6068 epg-config mm-decode
mm-bodies mm-encode mailabbrev gmm-utils mailheader sendmail sh-script
smie files-x shell-command+ shell pcomplete gnutls network-stream
url-http url-gw nsm shortdoc help-fns radix-tree textsec uni-scripts
idna-mapping uni-confusable textsec-check ucs-normalize shr-color
mail-parse rfc2231 rfc2047 rfc2045 ietf-drums url-cache url-auth eww xdg
url-queue shr pixel-fill kinsoku url-file svg puny mm-url gnus nnheader
gnus-util mail-utils range mm-util mail-prsvr executable recentf
tree-widget vc-backup log-view pcvs-util vc diff pulse color avy
misearch multi-isearch noutline outline vc-git diff-mode vc-dispatcher
bug-reference cursor-sensor face-remap server paredit edmacro kmacro
eros time-date checkdoc lisp-mnt flymake-proc flymake project warnings
thingatpt wordel-autoloads sokoban-autoloads ement-autoloads
svg-lib-autoloads taxy-magit-section-autoloads magit-section-autoloads
dash-autoloads taxy-autoloads plz-autoloads nov-autoloads
esxml-autoloads kv-autoloads transmission-autoloads lua-mode-autoloads
nix-mode-autoloads gnuplot-autoloads racket-mode-autoloads
eros-autoloads flymake-shellcheck-autoloads writegood-mode-autoloads
siege-mode-autoloads paredit-autoloads puni-autoloads
expand-region-autoloads filladapt-autoloads compose scroll-other-window
org-pdftools-autoloads org-noter-autoloads change-env-autoloads
math-delimiters-autoloads doct-autoloads ob-async-autoloads
async-autoloads emacs-ob-racket-autoloads valign-autoloads
cdlatex-autoloads auctex-autoloads tex-site pdf-occur ibuf-ext ibuffer
ibuffer-loaddefs tablist advice tablist-filter semantic/wisent/comp
semantic/wisent semantic/wisent/wisent semantic/util-modes semantic/util
semantic semantic/tag semantic/lex semantic/fw mode-local find-func
cedet pdf-isearch let-alist pdf-misc imenu pdf-tools package browse-url
url url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source eieio eieio-core json map byte-opt url-vars compile comint
osc ansi-color cus-edit icons wid-edit pdf-view password-cache jka-compr
pdf-cache pdf-info tq pdf-util pdf-macs image-mode dired-x dired
dired-loaddefs exif pdf-tools-autoloads tablist-autoloads mb-depth
repeat visual-fill-autoloads olivetti-autoloads time format-spec battery
dbus filenotify xml dom tamil99 quail disp-table lacarte-autoloads
shell-command-plus-autoloads winner ring delsel easy-mmode xt-mouse
cus-load avy-autoloads finder-inf vc-backup-autoloads compat-autoloads
icalendar diary-lib diary-loaddefs cal-menu calendar cal-loaddefs
filecache imenu-xref-autoloads derived chemtable-autoloads
molar-mass-autoloads rx saveplace-pdf-view saveplace bookmark
text-property-search pp saveplace-pdf-view-autoloads pcase
inspector-autoloads xr-autoloads straight-autoloads cl-seq info cl-extra
help-mode straight subr-x cl-macs gv cl-loaddefs cl-lib bytecomp
byte-compile cconv vz-nh-theme vz-options-theme rmc iso-transl tooltip
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
faces cus-face macroexp files window text-properties overlay sha1 md5
base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo x-toolkit
xinput2 x multi-tty make-network-process emacs)

Memory information:
((conses 16 616972 46438)
 (symbols 48 23834 4)
 (strings 32 146028 7174)
 (string-bytes 1 13261806)
 (vectors 16 77190)
 (vector-slots 8 1720887 205584)
 (floats 8 378 535)
 (intervals 56 73815 211)
 (buffers 1000 37))

bug#58118: 29.0.50; Follow #target links in eww without re-rendering page

[Lars Ingebrigtsen]

Visuwesh <visuweshm@gmail.com> writes:

> eww-follow-link currently follows #target links in the same URL by
> re-rendering the page.  Whilst this is fine for small HTML files, the
> 1.9M HTML file that I'm currently visiting takes a couple seconds to
> render.

That's a regression, I think -- following #target links used to work
without re-rendering.  (At least it did at one point.)

> I see that eww-display-html follows #target links by a text property
> search, why don't we do the same in eww-follow-link as well?  I have one
> question though: what do we about non-existent #targets?  Chrome and
> Firefox seem to not scroll when I add a non-existent #target to the
> current URL and say RET, and it seems to specially handle #top to mean
> to go to the top even if the HTML has no hits for "#top".  Should we
> handle the special #top target?

Sure, makes sense.

> +        (setq match (text-property-search-forward 'shr-target-id target #'member))
> +        (if match
> +            (goto-char (prop-match-beginning match))

This is more conveniently expressed as

(when-let ((text-property-search-forward 'shr-target-id target #'member))
  (goto-char (prop-match-beginning match)))

bug#58118: 29.0.50; Follow #target links in eww without re-rendering page

[Visuwesh]

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

[செவ்வாய் செப்டம்பர் 27, 2022] Lars Ingebrigtsen wrote:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> eww-follow-link currently follows #target links in the same URL by
>> re-rendering the page.  Whilst this is fine for small HTML files, the
>> 1.9M HTML file that I'm currently visiting takes a couple seconds to
>> render.
>
> That's a regression, I think -- following #target links used to work
> without re-rendering.  (At least it did at one point.)

That's what I seem to remember as well.

>> I see that eww-display-html follows #target links by a text property
>> search, why don't we do the same in eww-follow-link as well?  I have one
>> question though: what do we about non-existent #targets?  Chrome and
>> Firefox seem to not scroll when I add a non-existent #target to the
>> current URL and say RET, and it seems to specially handle #top to mean
>> to go to the top even if the HTML has no hits for "#top".  Should we
>> handle the special #top target?
>
> Sure, makes sense.

Now done.

>> +        (setq match (text-property-search-forward 'shr-target-id target #'member))
>> +        (if match
>> +            (goto-char (prop-match-beginning match))
>
> This is more conveniently expressed as
>
> (when-let ((text-property-search-forward 'shr-target-id target #'member))
>   (goto-char (prop-match-beginning match)))

Thanks.  Revised patch attached,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Follow-target-links-in-eww-without-re-rendering.patch --]
[-- Type: text/x-diff, Size: 2127 bytes --]

From 4b7280340de897137e1f4272219dddde292e5a4b Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Wed, 28 Sep 2022 08:11:42 +0530
Subject: [PATCH] Follow #target links in eww without re-rendering

* lisp/net/eww.el (eww-follow-link): Do a text property search instead
of re-rendering to follow #target links in the same page.  (bug#58118)
(eww-link-keymap): Bind <mouse-2> to eww-follow-link as well.
---
 lisp/net/eww.el | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 61b1b75068..624958915a 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -315,7 +315,8 @@ eww-accept-content-types
 
 (defvar-keymap eww-link-keymap
   :parent shr-map
-  "RET" #'eww-follow-link)
+  "RET" #'eww-follow-link
+  "<mouse-2>" #'eww-follow-link)
 
 (defvar-keymap eww-image-link-keymap
   :parent shr-map
@@ -1901,7 +1902,8 @@ eww-follow-link
    eww-mode)
   (mouse-set-point mouse-event)
   (let* ((orig-url (get-text-property (point) 'shr-url))
-         (url (eww--transform-url orig-url)))
+         (url (eww--transform-url orig-url))
+         target)
     (cond
      ((not url)
       (message "No link under point"))
@@ -1913,12 +1915,17 @@ eww-follow-link
       (funcall browse-url-secondary-browser-function url)
       (shr--blink-link))
      ;; This is a #target url in the same page as the current one.
-     ((and (url-target (url-generic-parse-url url))
+     ((and (setq target (url-target (url-generic-parse-url url)))
 	   (eww-same-page-p url (plist-get eww-data :url)))
-      (let ((dom (plist-get eww-data :dom)))
+      (let ((point (point)))
 	(eww-save-history)
 	(plist-put eww-data :url url)
-	(eww-display-html 'utf-8 url dom nil (current-buffer))))
+        (goto-char (point-min))
+        (if-let ((match (text-property-search-forward 'shr-target-id target #'member)))
+            (goto-char (prop-match-beginning match))
+          (goto-char (if (equal target "top")
+                         (point-min)
+                       point)))))
      (t
       (eww-browse-url orig-url external)))))
 
-- 
2.35.1

bug#58118: 29.0.50; Follow #target links in eww without re-rendering page

[Lars Ingebrigtsen]

Visuwesh <visuweshm@gmail.com> writes:

> Thanks.  Revised patch attached,

Thanks; pushed to Emacs 29.

bug#58118: 29.0.50; Follow #target links in eww without re-rendering page

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Visuwesh <visuweshm@gmail.com> writes:
>
>> Thanks.  Revised patch attached,
>
> Thanks; pushed to Emacs 29.

This was never closed; closing now.