bug#40532: 28.0.50; eww/shr: Anchor link does not work

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Arnaud Fontaine]

Trying to  open an anchor  link invariably goes back  to the top  of the
page instead of going to the specific point on the page. Example:
  1. emacs -Q
  2. M-x eww => https://en.wikipedia.org/wiki/Emacs
  3. Click or press enter on `1. History` anchor link of the TOC (target: https://en.wikipedia.org/wiki/Emacs#History)
  4. This goes to the top of the page rather than the `History` section.

This problem is not  recent and I have had this issue  for some time now
(I use Emacs Git snapshot).

Any help appreciated. Thanks!


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.12, cairo version 1.14.8)
 of 2020-03-29 built on marvin
Repository revision: 52fab66c277cd8d83fad0bd6bda8234e102bdc02
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11907000
System Description: Hyperbola GNU/Linux-libre

Recent messages:
Quit
Mark set
b is undefined
scroll-down-command: Beginning of buffer

Mark set
Mark saved where search started
uncompressing eww.el.gz...done
Note: file is write protected
Mark set

Configured using:
 'configure --prefix=/opt/emacs-20200329-52fab66c'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11 XDBE XIM MODULES THREADS PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=fcitx
  locale-coding-system: utf-8-unix

Major mode: ELisp/l

Minor modes in effect:
  shell-dirtrack-mode: t
  jabber-activity-mode: t
  desktop-save-mode: t
  savehist-mode: t
  window-numbering-mode: t
  show-paren-mode: t
  global-hl-line-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow eieio-opt speedbar ezimage dframe emacsbug gnus-draft org-id
tabify org-mobile cal-move mhtml-mode css-mode color js imenu cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs goto-addr pp sh-script smie executable misearch multi-isearch
ox-org ox-odt rng-loc rng-uri rng-parse rng-match rng-dt rng-util
rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util ox-latex
ox-icalendar ox-html table ox-ascii ox-publish ox sendmail help-fns
radix-tree cl-print debug backtrace jabber-rtt url-http url-gw url-cache
url-auth jabber-keepalive jabber-ping smiley gnus-cite mm-archive
mail-extr gnus-bcklg qp gnus-async gnus-ml disp-table gnus-topic nndraft
nnmh gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-cache
nnfolder utf-7 network-stream nsm nntp spam spam-stat gnus-uu yenc
gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-alias
imap rfc2104 nnrss bbdb-gnus bbdb-mua bbdb-com crm cl-extra help-mode
autorevert filenotify eww mm-url thingatpt url-queue vc-git diff-mode
cap-words superword subword python tramp-sh tramp tramp-loaddefs
trampver tramp-integration files-x tramp-compat shell ls-lisp jabber
jabber-notifications notifications jabber-libnotify dbus jabber-awesome
jabber-osd jabber-wmii jabber-xmessage jabber-festival jabber-sawfish
jabber-ratpoison jabber-tmux jabber-screen jabber-socks5
jabber-ft-server jabber-si-server jabber-ft-client jabber-ft-common
jabber-si-client jabber-si-common jabber-feature-neg jabber-truncate
jabber-time jabber-autoaway jabber-vcard-avatars jabber-chatstates
jabber-events jabber-vcard jabber-avatar jabber-activity jabber-watch
jabber-modeline jabber-ahc-presence jabber-ahc jabber-version
jabber-ourversion jabber-muc-nick-completion hippie-exp jabber-browse
jabber-search jabber-register jabber-roster jabber-presence jabber-muc
jabber-bookmarks jabber-private jabber-muc-nick-coloring hexrgb
jabber-widget jabber-disco jabber-chat jabber-history jabber-chatbuffer
jabber-alert jabber-iq jabber-core jabber-console sgml-mode ewoc
jabber-keymap jabber-sasl sasl sasl-anonymous sasl-login sasl-plain fsm
jabber-logon jabber-conn srv dns starttls tls gnutls jabber-xml
jabber-menu jabber-util tsdh-dark-theme desktop frameset
emms-librefm-stream emms-librefm-scrobbler emms-playlist-limit
emms-volume emms-volume-mixerctl emms-volume-pulse emms-volume-amixer
emms-i18n emms-history emms-score emms-stream-info
emms-metaplaylist-mode emms-bookmarks emms-cue emms-mode-line-icon
emms-browser sort emms-playlist-sort emms-last-played emms-player-xine
emms-player-mpd tq emms-playing-time emms-lyrics emms-url emms-streams
emms-show-all emms-tag-editor emms-mark emms-mode-line emms-cache
emms-info-opusinfo emms-info-ogginfo emms-info-mp3info
emms-playlist-mode emms-player-vlc emms-player-mpv emms-player-mplayer
emms-player-simple emms-source-playlist emms-source-file locate
emms-info-libtag emms-info later-do emms-setup emms emms-compat cal-iso
solar cal-dst diary-lib diary-loaddefs org-indent org-element avl-tree
ol-eww ol-rmail ol-mhe ol-irc ol-info ol-gnus nnir gnus-sum url
url-proxy url-privacy url-expand url-methods url-history mailcap shr
url-cookie url-domsuf url-util svg xml dom gnus-group gnus-undo
gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo
parse-time iso8601 gnus-spec gnus-int gnus-range message rmc puny rfc822
mml mml-sec epa derived epg epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader gnus-win gnus
nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums
text-property-search mail-utils mm-util mail-prsvr wid-edit ol-docview
doc-view jka-compr image-mode exif dired dired-loaddefs ol-bibtex bibtex
ol-bbdb ol-w3m org-agenda org-capture org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete
pcomplete comint ansi-color org-list org-faces org-entities time-date
noutline outline org-version ob-emacs-lisp ob-core ob-eval org-table ol
org-keys org-compat org-macs org-loaddefs format-spec find-func bbdb
bbdb-site timezone japanese-holidays holidays hol-loaddefs cal-menu
calendar cal-loaddefs savehist etags fileloop generator xref project
ring ido mule-util alist pym static apel-ver product elscreen
window-numbering easy-mmode cl avoid paren whitespace iso-transl edmacro
kmacro server mozc-isearch advice hl-line finder-inf mozc info package
easymenu browse-url url-handlers url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
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 elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic 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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 1710522 112857)
 (symbols 48 42342 64)
 (strings 32 324435 27012)
 (string-bytes 1 10738281)
 (vectors 16 76001)
 (vector-slots 8 1682075 266556)
 (floats 8 1026 2335)
 (intervals 56 166551 3077)
 (buffers 1000 191))
-- 
Arnaud

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Arnaud Fontaine]

Hi,

I have  investigated a little  and it  seems to be  because of a  bug in
eww-follow-link (bound to RET key).

It goes to the anchor link by clicking on it (<mouse-2>) or with 'v' key
as it  actually calls  shr-browse-url (but this  has the  side-effect of
reloading the whole page though).

Cheers,
-- 
Arnaud

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

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

found 40532 27.0.50
tags 40532 + patch
quit

Arnaud Fontaine <arnau@mini-dweeb.org> writes:

> I have  investigated a little  and it  seems to be  because of a  bug in
> eww-follow-link (bound to RET key).

Indeed; I think it's a regression in Emacs 27 caused by the following
fix for bug#28441:

Make #anchors work again in eww
fa41693799 2018-04-13 14:55:55 +0200
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=fa416937997a113d84ab4e4910d730ce5d77613d

Previously, eww-follow-link depended on eww-display-html to set
shr-target-id, but following this change that's no longer the case.

This doesn't affect eww-reload, which also calls eww-display-html,
because it passes it an explicit position to jump to.

> It goes to the anchor link by clicking on it (<mouse-2>) or with 'v' key
> as it  actually calls  shr-browse-url (but this  has the  side-effect of
> reloading the whole page though).

Yes, we wouldn't want eww-follow-link to do that, and in fact it already
contains logic to avoid reloading the whole page, modulo the
aforementioned regression in Emacs 27.

Lars, Eli, how's the following fix for emacs-27?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-following-target-on-same-page-in-EWW.patch --]
[-- Type: text/x-diff, Size: 1072 bytes --]

From 213264081a827e9041dbab294a5ff72fdc71b45f Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Wed, 22 Apr 2020 11:42:17 +0100
Subject: [PATCH] Fix following target on same page in EWW

* lisp/net/eww.el (eww-follow-link): Set shr-target-id before
calling eww-display-html as the latter no longer does so for
us (bug#28441, bug#40532).
---
 lisp/net/eww.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index c83884fd25..8bbbcae9c2 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -1578,7 +1578,8 @@ eww-follow-link
      ;; This is a #target url in the same page as the current one.
      ((and (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))
+            (shr-target-id (url-target (url-generic-parse-url url))))
 	(eww-save-history)
 	(plist-put eww-data :url url)
 	(eww-display-html 'utf-8 url dom nil (current-buffer))))
-- 
2.26.1


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


Thanks,

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

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

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Indeed; I think it's a regression in Emacs 27 caused by the following
> fix for bug#28441:
>
> Make #anchors work again in eww
> fa41693799 2018-04-13 14:55:55 +0200
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=fa416937997a113d84ab4e4910d730ce5d77613d
>
> Previously, eww-follow-link depended on eww-display-html to set
> shr-target-id, but following this change that's no longer the case.
>
> This doesn't affect eww-reload, which also calls eww-display-html,
> because it passes it an explicit position to jump to.

[...]

> Lars, Eli, how's the following fix for emacs-27?

Here's an even better fix for the regression IMO:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-eww-follow-link-on-URLs-with-target.patch --]
[-- Type: text/x-diff, Size: 1061 bytes --]

From 541a85fd17193a67883a36876fb982710abe4f7d Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Wed, 22 Apr 2020 11:42:17 +0100
Subject: [PATCH] Fix eww-follow-link on URLs with #target

* lisp/net/eww.el (eww-display-html): Ensure shr-target-id is set as
callers depend on this (bug#28441, bug#40532).
---
 lisp/net/eww.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index c83884fd25..1be499172b 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -518,6 +518,10 @@ eww-display-html
       (plist-put eww-data :dom document)
       (let ((inhibit-read-only t)
 	    (inhibit-modification-hooks t)
+            ;; Possibly set by the caller, e.g., `eww-render' which
+            ;; preserves the old URL #target before chasing redirects.
+            (shr-target-id (or shr-target-id
+                               (url-target (url-generic-parse-url url))))
 	    (shr-external-rendering-functions
              (append
               shr-external-rendering-functions
-- 
2.26.1


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


WDYT?

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Wed, 22 Apr 2020 12:38:17 +0100
> Cc: 40532@debbugs.gnu.org
> 
> Arnaud Fontaine <arnau@mini-dweeb.org> writes:
> 
> > I have  investigated a little  and it  seems to be  because of a  bug in
> > eww-follow-link (bound to RET key).
> 
> Indeed; I think it's a regression in Emacs 27 caused by the following
> fix for bug#28441:
> 
> Make #anchors work again in eww
> fa41693799 2018-04-13 14:55:55 +0200
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=fa416937997a113d84ab4e4910d730ce5d77613d

Then how come I cannot reproduce it in today's master?

> Yes, we wouldn't want eww-follow-link to do that, and in fact it already
> contains logic to avoid reloading the whole page, modulo the
> aforementioned regression in Emacs 27.
> 
> Lars, Eli, how's the following fix for emacs-27?

I need first to understand what is going on, and in particular why it
doesn't happen to me.

Thanks.

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Wed, 22 Apr 2020 12:38:17 +0100
>> Cc: 40532@debbugs.gnu.org
>> 
>> Arnaud Fontaine <arnau@mini-dweeb.org> writes:
>> 
>> > I have  investigated a little  and it  seems to be  because of a  bug in
>> > eww-follow-link (bound to RET key).
>> 
>> Indeed; I think it's a regression in Emacs 27 caused by the following
>> fix for bug#28441:
>> 
>> Make #anchors work again in eww
>> fa41693799 2018-04-13 14:55:55 +0200
>> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=fa416937997a113d84ab4e4910d730ce5d77613d
>
> Then how come I cannot reproduce it in today's master?

0. emacs -Q
1. M-x eww RET https://en.wikipedia.org/wiki/Emacs RET
2. C-s Cul RET (place point on ToC entry "3 Culture")
3. RET         (eww-follow-link)

Expected: Point jumps to heading "Culture[edit]" at pos 21205 on L324
Observed: Point jumps to point-min
Emacs version details follow my signature.
What do you see on your end?

>> Yes, we wouldn't want eww-follow-link to do that, and in fact it already
>> contains logic to avoid reloading the whole page, modulo the
>> aforementioned regression in Emacs 27.
>> 
>> Lars, Eli, how's the following fix for emacs-27?
>
> I need first to understand what is going on, and in particular why it
> doesn't happen to me.

What's going on: shr.el currently requires its users to set
shr-target-id before rendering a DOM, so that it knows where to place
the eponymous text property.

The only user of this text property in the Emacs sources is
eww-display-html, which leaves point where the text property was placed
after shr is done rendering the DOM.

Prior to the patch for bug#28441, eww-display-html would always set
shr-target-id to the current url-target (or nil) prior to invoking shr.

The patch for bug#28441 removed the setting of shr-target-id from
eww-display-html and moved it to eww-render, so that it's set before any
URL redirections strip the original url-target away.

Since eww-follow-link calls eww-display-html directly, rather than going
through eww-render (which is a url-retrieve callback), shr-target-id is
no longer set appropriately when eww-follow-link is invoked on a URL
with a #target.  So shr.el doesn't know to insert the corresponding text
property, and eww-display-html leaves point at point-min rather than the
correct #target.

-- 
Basil

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw3d scroll bars)
 of 2020-04-22 built on thunk
Repository revision: ab214143bbc633bcbe1ae146647c2fdc882122f0
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux bullseye/sid

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Contacting host: en.wikipedia.org:443
uncompressing publicsuffix.txt.gz...done
Mark saved where search started

Configured using:
 'configure 'CC=ccache gcc' 'CFLAGS=-O2 -march=native' --config-cache
 --prefix=/home/blc/.local --with-x-toolkit=lucid
 --with-file-notification=yes --with-x'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB
NOTIFY INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT
LIBOTF ZLIB TOOLKIT_SCROLL_BARS LUCID X11 XDBE XIM MODULES THREADS
LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_IE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: eww

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired dired-loaddefs rfc822 mml
mml-sec epa derived epg epg-config mm-decode mm-bodies mm-encode
mailabbrev gmm-utils mailheader sendmail misearch multi-isearch
shr-color color cl-extra help-mode jka-compr gnutls network-stream
url-http mail-parse rfc2231 url-gw nsm rmc url-cache url-auth eww
easymenu mm-url gnus nnheader gnus-util rmail rmail-loaddefs rfc2047
rfc2045 ietf-drums time-date mail-utils wid-edit mm-util mail-prsvr
thingatpt url-queue url url-proxy url-privacy url-expand url-methods
url-history mailcap shr text-property-search url-cookie url-domsuf
url-util url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars puny svg xml seq
byte-opt gv bytecomp byte-compile cconv dom browse-url format-spec
cl-loaddefs cl-lib tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type 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 elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 289451 9169)
 (symbols 48 9994 1)
 (strings 32 49262 2043)
 (string-bytes 1 2896806)
 (vectors 16 16043)
 (vector-slots 8 290544 20084)
 (floats 8 137 326)
 (intervals 56 32031 257)
 (buffers 992 11))

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: arnau@mini-dweeb.org,  larsi@gnus.org,  40532@debbugs.gnu.org
> Date: Wed, 22 Apr 2020 16:44:18 +0100
> 
> > Then how come I cannot reproduce it in today's master?
> 
> 0. emacs -Q
> 1. M-x eww RET https://en.wikipedia.org/wiki/Emacs RET
> 2. C-s Cul RET (place point on ToC entry "3 Culture")
> 3. RET         (eww-follow-link)

So one _must_ press RET.  the original report said "Click or press
enter", so I did the former.

Thanks for the rest, I will look into it.

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: arnau@mini-dweeb.org,  larsi@gnus.org,  40532@debbugs.gnu.org
>> Date: Wed, 22 Apr 2020 16:44:18 +0100
>> 
>> > Then how come I cannot reproduce it in today's master?
>> 
>> 0. emacs -Q
>> 1. M-x eww RET https://en.wikipedia.org/wiki/Emacs RET
>> 2. C-s Cul RET (place point on ToC entry "3 Culture")
>> 3. RET         (eww-follow-link)
>
> So one _must_ press RET.  the original report said "Click or press
> enter", so I did the former.

Right, that can't be true, since "v" and <mouse-2> invoke
shr-browse-url, which refetches the page and thus goes through the
eww-render route.

> Thanks for the rest, I will look into it.

Thanks,

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: arnau@mini-dweeb.org,  larsi@gnus.org,  40532@debbugs.gnu.org
> Date: Wed, 22 Apr 2020 17:15:11 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> 0. emacs -Q
> >> 1. M-x eww RET https://en.wikipedia.org/wiki/Emacs RET
> >> 2. C-s Cul RET (place point on ToC entry "3 Culture")
> >> 3. RET         (eww-follow-link)
> >
> > So one _must_ press RET.  the original report said "Click or press
> > enter", so I did the former.
> 
> Right, that can't be true, since "v" and <mouse-2> invoke
> shr-browse-url, which refetches the page and thus goes through the
> eww-render route.

I clicked mouse-1, not mouse-2.

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Arnaud Fontaine]

Hi,

Basil L. Contovounesios <contovob@tcd.ie> writes:
> Here's an even better fix for the regression IMO:
>
> From 541a85fd17193a67883a36876fb982710abe4f7d Mon Sep 17 00:00:00 2001
> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Wed, 22 Apr 2020 11:42:17 +0100
> Subject: [PATCH] Fix eww-follow-link on URLs with #target

I can confirm that this fixes the issue, thank you very much!

Eli Zaretskii <eliz@gnu.org> writes:
> So one _must_ press RET.  the original report said "Click or press
> enter", so I did the former.
>
> Thanks for the rest, I will look into it.

Sorry, my initial report was misleading and not accurate enough.

Cheers,
-- 
Arnaud

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

[ Sorry, Eli, for accidentally sending this to you directly. ]

Eli Zaretskii <eliz@gnu.org> writes:

>> > So one _must_ press RET.  the original report said "Click or press
>> > enter", so I did the former.
>> 
>> Right, that can't be true, since "v" and <mouse-2> invoke
>> shr-browse-url, which refetches the page and thus goes through the
>> eww-render route.
>
> I clicked mouse-1, not mouse-2.

I also clicked mouse-1, but it's translated to mouse-2.  Of the two,
only the latter is bound in shr-map and its derivative eww-link-keymap.

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Arnaud Fontaine <arnau@mini-dweeb.org> writes:

> Basil L. Contovounesios <contovob@tcd.ie> writes:
>> Here's an even better fix for the regression IMO:
>>
>> From 541a85fd17193a67883a36876fb982710abe4f7d Mon Sep 17 00:00:00 2001
>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Wed, 22 Apr 2020 11:42:17 +0100
>> Subject: [PATCH] Fix eww-follow-link on URLs with #target
>
> I can confirm that this fixes the issue, thank you very much!

Thanks for testing.

> Eli Zaretskii <eliz@gnu.org> writes:
>> So one _must_ press RET.  the original report said "Click or press
>> enter", so I did the former.
>>
>> Thanks for the rest, I will look into it.
>
> Sorry, my initial report was misleading and not accurate enough.

No worries.

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Eli Zaretskii]

> Date: Wed, 22 Apr 2020 18:57:07 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: larsi@gnus.org, 40532@debbugs.gnu.org, arnau@mini-dweeb.org
> 
> > From: "Basil L. Contovounesios" <contovob@tcd.ie>
> > Cc: arnau@mini-dweeb.org,  larsi@gnus.org,  40532@debbugs.gnu.org
> > Date: Wed, 22 Apr 2020 16:44:18 +0100
> > 
> > > Then how come I cannot reproduce it in today's master?
> > 
> > 0. emacs -Q
> > 1. M-x eww RET https://en.wikipedia.org/wiki/Emacs RET
> > 2. C-s Cul RET (place point on ToC entry "3 Culture")
> > 3. RET         (eww-follow-link)
> 
> So one _must_ press RET.  the original report said "Click or press
> enter", so I did the former.
> 
> Thanks for the rest, I will look into it.

I think this is OK for the release branch, but I'd like Lars to
comment.  Lars, please chime in.

Thanks.

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Arnaud Fontaine]

Hi,

> Here's an even better fix for the regression IMO:
>
> From 541a85fd17193a67883a36876fb982710abe4f7d Mon Sep 17 00:00:00 2001
> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Wed, 22 Apr 2020 11:42:17 +0100
> Subject: [PATCH] Fix eww-follow-link on URLs with #target

While wikipedia  works fine  with your  patch, I  have noticed  that the
following does not work though:
  0. emacs -Q
  1. M-x eww RET https://distrowatch.com/weekly.php?issue=20200413 RET
  2. C-s HoleOS RET (place point on ToC entry "New distributions: HoleOS")
  3. RET (eww-follow link)

This goes to the top of the page.

Cheers,
-- 
Arnaud

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Arnaud Fontaine <arnau@mini-dweeb.org> writes:

>> Here's an even better fix for the regression IMO:
>>
>> From 541a85fd17193a67883a36876fb982710abe4f7d Mon Sep 17 00:00:00 2001
>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Date: Wed, 22 Apr 2020 11:42:17 +0100
>> Subject: [PATCH] Fix eww-follow-link on URLs with #target
>
> While wikipedia  works fine  with your  patch, I  have noticed  that the
> following does not work though:
>   0. emacs -Q
>   1. M-x eww RET https://distrowatch.com/weekly.php?issue=20200413 RET
>   2. C-s HoleOS RET (place point on ToC entry "New distributions: HoleOS")
>   3. RET (eww-follow link)
>
> This goes to the top of the page.

Indeed, I've noticed this issue before, but it's a separate issue to the
regression we've been discussing so far.

Here's a more easily reproducible recipe if you build Emacs from its
source repository (if I visit the given distrowatch site using Emacs 25
or 26 for some reason HoleOS is not mentioned):

0. ./src/emacs -Q
1. M-x eww-open-file RET admin/unidata/copyright.html RET
2. C-s misc RET RET

This jumps to point-min rather than the Miscellaneous heading, and is
reproducible since at least Emacs 25.

The problem is with relative targets: for some reason shr-target-id is
not set appropriately when shr is rendering the page, so no
shr-target-id text property gets set, and eww can't subsequently find
anywhere to jump to.

Perhaps shr should unconditionally (without requiring shr-target-id to
be bound) mark all possible targets with text properties during
rendering.  I'll see if I can get this or any other way working today or
tomorrow, but either way this won't be fixed in Emacs 27.

Thanks,

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Perhaps shr should unconditionally (without requiring shr-target-id to
> be bound) mark all possible targets with text properties during
> rendering.  I'll see if I can get this or any other way working today or
> tomorrow, but either way this won't be fixed in Emacs 27.

Hm...  marking all the targets sounds like overkill (if I understand you
correctly, but it's likely that I don't).  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I think this is OK for the release branch, but I'd like Lars to
> comment.  Lars, please chime in.

I think this looks correct:

-      (let ((dom (plist-get eww-data :dom)))
+      (let ((dom (plist-get eww-data :dom))
+            (shr-target-id (url-target (url-generic-parse-url url))))

But I haven't tested the code.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> I think this is OK for the release branch, but I'd like Lars to
>> comment.  Lars, please chime in.
>
> I think this looks correct:
>
> -      (let ((dom (plist-get eww-data :dom)))
> +      (let ((dom (plist-get eww-data :dom))
> +            (shr-target-id (url-target (url-generic-parse-url url))))
>
> But I haven't tested the code.

Why not move this binding back into eww-display-html instead, closer to
where shr-insert-document is called, for the benefit of other callers of
eww-display-html as well, and deviating the least from previous versions
of eww, as per the second proposed patch?

Thanks,

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Perhaps shr should unconditionally (without requiring shr-target-id to
>> be bound) mark all possible targets with text properties during
>> rendering.  I'll see if I can get this or any other way working today or
>> tomorrow, but either way this won't be fixed in Emacs 27.
>
> Hm...  marking all the targets sounds like overkill (if I understand you
> correctly, but it's likely that I don't).  :-)

Marking all the targets is what I meant, but I was only thinking aloud,
not seriously suggesting it.  I'm still stepping through eww and shr
trying to figure out why shr-target-id doesn't work sometimes (my
current guess is shr modifies the DOM to avoid re-rendering tables, so
shr-target-id doesn't get set in such cases); progress is slow.  :(

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Why not move this binding back into eww-display-html instead, closer to
> where shr-insert-document is called, for the benefit of other callers of
> eww-display-html as well, and deviating the least from previous versions
> of eww, as per the second proposed patch?

Sorry, I missed that patch.  That looks even more logical to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Why not move this binding back into eww-display-html instead, closer to
>> where shr-insert-document is called, for the benefit of other callers of
>> eww-display-html as well, and deviating the least from previous versions
>> of eww, as per the second proposed patch?
>
> Sorry, I missed that patch.  That looks even more logical to me.

Thanks, pushed to emacs-27:

Fix eww-follow-link on URLs with #target
310112fdc7 2020-05-04 00:40:38 +0100
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=310112fdc7448a9297085333fcd4bf4088e634bf

But unless I hear otherwise I'll hold off from closing this bug until I
can look more into the relative #target issue tomorrow.

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Arnaud Fontaine]

Hi,

> Thanks, pushed to emacs-27:
>
> Fix eww-follow-link on URLs with #target
> 310112fdc7 2020-05-04 00:40:38 +0100
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=310112fdc7448a9297085333fcd4bf4088e634bf

Great, thank you so much!

> But unless I hear otherwise I'll hold off from closing this bug until I
> can look more into the relative #target issue tomorrow.

Thanks.

Cheers,
-- 
Arnaud

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

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

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>>
>>> Perhaps shr should unconditionally (without requiring shr-target-id to
>>> be bound) mark all possible targets with text properties during
>>> rendering.  I'll see if I can get this or any other way working today or
>>> tomorrow, but either way this won't be fixed in Emacs 27.
>>
>> Hm...  marking all the targets sounds like overkill (if I understand you
>> correctly, but it's likely that I don't).  :-)
>
> Marking all the targets is what I meant, but I was only thinking aloud,
> not seriously suggesting it.  I'm still stepping through eww and shr
> trying to figure out why shr-target-id doesn't work sometimes (my
> current guess is shr modifies the DOM to avoid re-rendering tables, so
> shr-target-id doesn't get set in such cases); progress is slow.  :(

Now that I understand what's happening better, I think it's necessary to
mark all targets, and I don't think it's overkill.

The problem is indeed that shr caches rendered tables.  What this means
is that, if eww first visits a URL with no #target, then its tables will
be rendered and cached with no shr-target-id text property attached to
any fragment identifiers contained within them.

If eww-follow-link is subsequently invoked on a #target within the same
page, then the cached table contents will be inserted and so there will
be no shr-target-id property in the buffer.  The recipe provided in [1]
illustrates this.

[1]: https://debbugs.gnu.org/40532#51

OTOH if the shr-target-id property is always attached to the relevant
'id' and (deprecated) 'name' attributes, then cached table contents will
still be searchable.  This shouldn't be overkill in terms of performance
because yet another text property on a subet of the DOM should be
comparatively cheap, right?

So WDYT of the following fix?  Can you think of any better solutions?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Propertize-all-shr-fragment-IDs-as-shr-target-id.patch --]
[-- Type: text/x-diff, Size: 4090 bytes --]

From ef0058cb4a70b1d78e55f6b61ff0e1e8ffad9169 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Fri, 8 May 2020 00:25:38 +0100
Subject: [PATCH] Propertize all shr fragment IDs as shr-target-id

* lisp/net/shr.el (shr-target-id): Add docstring.
(shr-descend, shr-tag-a): Display dummy anchor characters as the
empty string.  Give all relevant 'id' or 'name' fragment identifier
attributes the shr-target-id text property.  This ensures that
cached content, such as tables, retains the property across
renders.  (Bug#40532)

* lisp/net/eww.el (eww-display-html): Adapt shr-target-id property
search accordingly.
---
 lisp/net/eww.el | 19 ++++++++++---------
 lisp/net/shr.el | 26 ++++++++++++++------------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index a6c1abdbb1..acb7cc7e40 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -26,13 +26,14 @@
 
 (require 'cl-lib)
 (require 'format-spec)
-(require 'shr)
-(require 'url)
-(require 'url-queue)
-(require 'thingatpt)
 (require 'mm-url)
 (require 'puny)
-(eval-when-compile (require 'subr-x)) ;; for string-trim
+(require 'shr)
+(require 'text-property-search)
+(require 'thingatpt)
+(require 'url)
+(require 'url-queue)
+(eval-when-compile (require 'subr-x))
 
 (defgroup eww nil
   "Emacs Web Wowser"
@@ -543,10 +544,10 @@ eww-display-html
 	  (goto-char point))
 	 (shr-target-id
 	  (goto-char (point-min))
-	  (let ((point (next-single-property-change
-			(point-min) 'shr-target-id)))
-	    (when point
-	      (goto-char point))))
+          (let ((match (text-property-search-forward
+                        'shr-target-id shr-target-id t)))
+            (when match
+              (goto-char (prop-match-beginning match)))))
 	 (t
 	  (goto-char (point-min))
 	  ;; Don't leave point inside forms, because the normal eww
diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 1f80ab74db..ea174e5d77 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -185,13 +185,15 @@ shr-base
 (defvar shr-depth 0)
 (defvar shr-warning nil)
 (defvar shr-ignore-cache nil)
-(defvar shr-target-id nil)
 (defvar shr-table-separator-length 1)
 (defvar shr-table-separator-pixel-width 0)
 (defvar shr-table-id nil)
 (defvar shr-current-font nil)
 (defvar shr-internal-bullet nil)
 
+(defvar shr-target-id nil
+  "Target fragment identifier anchor.")
+
 (defvar shr-map
   (let ((map (make-sparse-keymap)))
     (define-key map "a" 'shr-show-alt-text)
@@ -531,13 +533,13 @@ shr-descend
                (funcall function dom))
               (t
                (shr-generic dom)))
-	(when (and shr-target-id
-		   (equal (dom-attr dom 'id) shr-target-id))
+        (when-let* ((id (dom-attr dom 'id)))
 	  ;; If the element was empty, we don't have anything to put the
 	  ;; anchor on.  So just insert a dummy character.
 	  (when (= start (point))
-	    (insert "*"))
-	  (put-text-property start (1+ start) 'shr-target-id shr-target-id))
+            (insert ?*)
+            (put-text-property (1- (point)) (point) 'display ""))
+          (put-text-property start (1+ start) 'shr-target-id id))
 	;; If style is set, then this node has set the color.
 	(when style
 	  (shr-colorize-region
@@ -1497,14 +1499,14 @@ shr-tag-a
 	(start (point))
 	shr-start)
     (shr-generic dom)
-    (when (and shr-target-id
-	       (equal (dom-attr dom 'name) shr-target-id))
-      ;; We have a zero-length <a name="foo"> element, so just
-      ;; insert...  something.
+    (when-let* ((id (or (dom-attr dom 'id)
+                        ;; Obsolete since HTML5.
+                        (dom-attr dom 'name))))
+      ;; We have an empty element, so just insert... something.
       (when (= start (point))
-	(shr-ensure-newline)
-	(insert " "))
-      (put-text-property start (1+ start) 'shr-target-id shr-target-id))
+        (insert ?\s)
+        (put-text-property (1- (point)) (point) 'display ""))
+      (put-text-property start (1+ start) 'shr-target-id id))
     (when url
       (shr-urlify (or shr-start start) (shr-expand-url url) title))))
 
-- 
2.26.2


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


Thanks,

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Arnaud Fontaine]

Hi,

> So WDYT of the following fix?  Can you think of any better solutions?

Just to confirm that with this patch and the latest Git revision, it now
works  fine on  both wikipedia  and distrowatch  pages I  have mentioned
before. Thank you very much.

Cheers,
-- 
Arnaud

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Lars Ingebrigtsen]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> OTOH if the shr-target-id property is always attached to the relevant
> 'id' and (deprecated) 'name' attributes, then cached table contents will
> still be searchable.  This shouldn't be overkill in terms of performance
> because yet another text property on a subet of the DOM should be
> comparatively cheap, right?
>
> So WDYT of the following fix?  Can you think of any better solutions?

The patch is a bit hard to read, because it seems to have a lot of
unrelated changes like:

> -(require 'shr)
> -(require 'url)
> -(require 'url-queue)
> -(require 'thingatpt)
>  (require 'mm-url)
>  (require 'puny)
> -(eval-when-compile (require 'subr-x)) ;; for string-trim
> +(require 'shr)
> +(require 'text-property-search)
> +(require 'thingatpt)
> +(require 'url)
> +(require 'url-queue)
> +(eval-when-compile (require 'subr-x))

and

> -    (when (and shr-target-id
> -	       (equal (dom-attr dom 'name) shr-target-id))
> -      ;; We have a zero-length <a name="foo"> element, so just
> -      ;; insert...  something.
> +    (when-let* ((id (or (dom-attr dom 'id)
> +                        ;; Obsolete since HTML5.
> +                        (dom-attr dom 'name))))
> +      ;; We have an empty element, so just insert... something.

and

> -	    (insert "*"))
> -	  (put-text-property start (1+ start) 'shr-target-id shr-target-id))
> +            (insert ?*)
> +            (put-text-property (1- (point)) (point) 'display ""))
> +          (put-text-property start (1+ start) 'shr-target-id id))

so I can't really make out what the changes you're making in this area is...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> The patch is a bit hard to read, because it seems to have a lot of
> unrelated changes like:
>
>> -(require 'shr)
>> -(require 'url)
>> -(require 'url-queue)
>> -(require 'thingatpt)
>>  (require 'mm-url)
>>  (require 'puny)
>> -(eval-when-compile (require 'subr-x)) ;; for string-trim
>> +(require 'shr)
>> +(require 'text-property-search)
>> +(require 'thingatpt)
>> +(require 'url)
>> +(require 'url-queue)
>> +(eval-when-compile (require 'subr-x))

This is just adding (require 'text-property-search) and removing a stale
comment.  The only unrelated change is the lexicographic reordering.

> and
>
>> -    (when (and shr-target-id
>> -	       (equal (dom-attr dom 'name) shr-target-id))
>> -      ;; We have a zero-length <a name="foo"> element, so just
>> -      ;; insert...  something.
>> +    (when-let* ((id (or (dom-attr dom 'id)
>> +                        ;; Obsolete since HTML5.
>> +                        (dom-attr dom 'name))))
>> +      ;; We have an empty element, so just insert... something.

This is not an unrelated change; I'm changing the condition from:

  (and shr-target-id
       (equal (dom-attr dom 'name) shr-target-id))

to:

  (or (dom-attr dom 'id)
      (dom-attr dom 'name))

and storing the result of the condition for later reuse.  The key thing
to note is that the 'name' attribute is obsolete in HTML5 and the 'id'
attribute is recommended instead, which is why I'm checking both.

Though, now that I think about it again, we could avoid checking the
'id' attribute in both shr-tag-a and shr-descend by instead writing:

  (when-let* ((id (unless (dom-attr dom 'id) ; Handled by `shr-descend'.
                    (dom-attr dom 'name))))  ; Obsolete since HTML5.

> and
>
>> -	    (insert "*"))
>> -	  (put-text-property start (1+ start) 'shr-target-id shr-target-id))
>> +            (insert ?*)
>> +            (put-text-property (1- (point)) (point) 'display ""))
>> +          (put-text-property start (1+ start) 'shr-target-id id))
>
> so I can't really make out what the changes you're making in this area is...

Sorry, I didn't imagine a patch touching 20-odd lines would be
problematic.  Here's the updated patch in as minimal a form as possible:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Propertize-all-shr-fragment-IDs-as-shr-target-id.patch --]
[-- Type: text/x-diff, Size: 3137 bytes --]

From 8cced1ac250078f2ea1cf1b82538c98621f7ca2f Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Thu, 21 May 2020 23:18:33 +0100
Subject: [PATCH] Propertize all shr fragment IDs as shr-target-id

* lisp/net/shr.el (shr-descend, shr-tag-a): Display dummy anchor
characters as the empty string.  Give all relevant 'id' or 'name'
fragment identifier attributes the shr-target-id text property.
This ensures that cached content, such as tables, retains the
property across renders.  (Bug#40532)

* lisp/net/eww.el (eww-display-html): Adapt shr-target-id property
search accordingly.
---
 lisp/net/eww.el |  7 ++++---
 lisp/net/shr.el | 18 +++++++++---------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index a6c1abdbb1..b5780a6685 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -27,6 +27,7 @@
 (require 'cl-lib)
 (require 'format-spec)
 (require 'shr)
+(require 'text-property-search)
 (require 'url)
 (require 'url-queue)
 (require 'thingatpt)
@@ -543,10 +544,10 @@ eww-display-html
 	  (goto-char point))
 	 (shr-target-id
 	  (goto-char (point-min))
-	  (let ((point (next-single-property-change
-			(point-min) 'shr-target-id)))
+	  (let ((point (text-property-search-forward
+			'shr-target-id shr-target-id t)))
 	    (when point
-	      (goto-char point))))
+	      (goto-char (prop-match-beginning point)))))
 	 (t
 	  (goto-char (point-min))
 	  ;; Don't leave point inside forms, because the normal eww
diff --git a/lisp/net/shr.el b/lisp/net/shr.el
index 1f80ab74db..55c0c1d8ad 100644
--- a/lisp/net/shr.el
+++ b/lisp/net/shr.el
@@ -531,13 +531,13 @@ shr-descend
                (funcall function dom))
               (t
                (shr-generic dom)))
-	(when (and shr-target-id
-		   (equal (dom-attr dom 'id) shr-target-id))
+	(when-let* ((id (dom-attr dom 'id)))
 	  ;; If the element was empty, we don't have anything to put the
 	  ;; anchor on.  So just insert a dummy character.
 	  (when (= start (point))
-	    (insert "*"))
-	  (put-text-property start (1+ start) 'shr-target-id shr-target-id))
+	    (insert "*")
+	    (put-text-property (1- (point)) (point) 'display ""))
+	  (put-text-property start (1+ start) 'shr-target-id id))
 	;; If style is set, then this node has set the color.
 	(when style
 	  (shr-colorize-region
@@ -1497,14 +1497,14 @@ shr-tag-a
 	(start (point))
 	shr-start)
     (shr-generic dom)
-    (when (and shr-target-id
-	       (equal (dom-attr dom 'name) shr-target-id))
+    (when-let* ((id (unless (dom-attr dom 'id) ; Handled by `shr-descend'.
+                      (dom-attr dom 'name))))  ; Obsolete since HTML5.
       ;; We have a zero-length <a name="foo"> element, so just
       ;; insert...  something.
       (when (= start (point))
-	(shr-ensure-newline)
-	(insert " "))
-      (put-text-property start (1+ start) 'shr-target-id shr-target-id))
+	(insert " ")
+	(put-text-property (1- (point)) (point) 'display ""))
+      (put-text-property start (1+ start) 'shr-target-id id))
     (when url
       (shr-urlify (or shr-start start) (shr-expand-url url) title))))
 
-- 
2.26.2


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


WDYT?  Thanks,

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

Arnaud Fontaine <arnau@mini-dweeb.org> writes:

>> So WDYT of the following fix?  Can you think of any better solutions?
>
> Just to confirm that with this patch and the latest Git revision, it now
> works  fine on  both wikipedia  and distrowatch  pages I  have mentioned
> before. Thank you very much.

Thanks for testing!

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Here's the updated patch in as minimal a form as possible:

Any objections to installing this?

-- 
Basil

bug#40532: 28.0.50; eww/shr: Anchor link does not work

[Basil L. Contovounesios]

tags 40532 fixed
close 40532 28.1
quit

Pushed to master; closing.

Propertize all shr fragment IDs as shr-target-id
3dd6b23cdf 2020-06-18 16:16:49 +0100
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=3dd6b23cdfa64bdff2bdc9e7fbf9844a2ed6cd8f

-- 
Basil