GNU bug report logs - #36475
[PATCH] Handle 'abbr' and 'acronym' tags in shr.el

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36475 in the body.
You can then email your comments to 36475 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Nicholas Drozd <nicholasdrozd <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Tue, 2 Jul 2019 09:06:43 -0500

[Message part 1 (text/plain, inline)]

I came across some `abbr` tags in the wild: check out the first
paragraph of http://mindprod.com/jgloss/unmainobfuscation.html. EWW
doesn't handle those, so here is a patch for that.

My understanding is that there is also an `acronym` tag which has been
deprecated in favor of `abbr`. I figured we might as well cover that
one too just in case.

I picked the `diary` face for abbreviations, but that choice didn't
have a lot of thought put into it. Whoever commits can choose
something else.

Let's try to get this merged <abbr title="as soon as possible">ASAP</abbr>.

[0001-Handle-abbr-and-acronym-tags-in-shr.el.patch (text/x-patch, attachment)]

Message #8 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 36475 <at> debbugs.gnu.org
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Tue, 02 Jul 2019 20:26:02 +0200

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

> +(defun shr-tag-abbr (dom)
> +  (let ((title (dom-attr dom 'title))
> +	(start (point)))
> +    (shr-generic dom)
> +    (shr-add-font start (point) 'diary)
> +    (add-text-properties
> +     start (point)
> +     (list
> +      'help-echo title
> +      'mouse-face 'highlight))))
> +
> +(defun shr-tag-acronym (dom)
> +  ;; `acronym' is deprecated in favor of `abbr'.
> +  (shr-tag-abbr dom))

Couldn't this be a defalias?

Best regards, Michael.

Message #11 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: Nicholas Drozd <nicholasdrozd <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 36475 <at> debbugs.gnu.org
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Tue, 2 Jul 2019 18:11:03 -0500

It could be, but I don't know that much would be gained. While a
defalias would be slightly terser, it would also stick out from the
surrounding defuns. I don't have a strong opinion either way. Are
there any non-style considerations?

Message #14 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: 36475 <at> debbugs.gnu.org
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Wed, 03 Jul 2019 08:42:26 +0200

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

Hi Nicholas,

> It could be, but I don't know that much would be gained. While a
> defalias would be slightly terser, it would also stick out from the
> surrounding defuns. I don't have a strong opinion either way. Are
> there any non-style considerations?

I have no strong opinion either, but using a defalias seems to me the
better choice. Maybe I'm biased due to the work on Tramp, where a large
part of the job is keeping backward compatibility with older
Emacsen. That's why I have a preference to defalias, whenever
possible. It needs less maintainership attention.

But it is your decision, of course.

Best regards, Michael.

Message #17 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: Nicholas Drozd <nicholasdrozd <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 36475 <at> debbugs.gnu.org
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Wed, 3 Jul 2019 15:17:24 -0500

Let's leave it as a defun for now then, and anyone who objects can
change it later (probably without any consequence). shr-tag-tt does
the same thing, deferring to shr-tag-code, so shr-tag-tt and
shr-tag-acronym can be changed together.

On Wed, Jul 3, 2019 at 1:42 AM Michael Albinus <michael.albinus <at> gmx.de> wrote:
>
> Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:
>
> Hi Nicholas,
>
> > It could be, but I don't know that much would be gained. While a
> > defalias would be slightly terser, it would also stick out from the
> > surrounding defuns. I don't have a strong opinion either way. Are
> > there any non-style considerations?
>
> I have no strong opinion either, but using a defalias seems to me the
> better choice. Maybe I'm biased due to the work on Tramp, where a large
> part of the job is keeping backward compatibility with older
> Emacsen. That's why I have a preference to defalias, whenever
> possible. It needs less maintainership attention.
>
> But it is your decision, of course.
>
> Best regards, Michael.

Message #20 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 36475 <at> debbugs.gnu.org
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Thu, 04 Jul 2019 20:40:12 +0100

severity 36475 wishlist
quit

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

> I came across some `abbr` tags in the wild: check out the first
> paragraph of http://mindprod.com/jgloss/unmainobfuscation.html. EWW
> doesn't handle those, so here is a patch for that.
>
> My understanding is that there is also an `acronym` tag which has been
> deprecated in favor of `abbr`. I figured we might as well cover that
> one too just in case.

+1.

> I picked the `diary` face for abbreviations, but that choice didn't
> have a lot of thought put into it. Whoever commits can choose
> something else.

Indeed I don't think the 'diary' face is a very good fit.  Not only is
it undefined unless calendar.el is loaded, but it also has a quite
jarring red/yellow foreground by default, and bears no relation to
abbreviations.

The major browsers use a dotted underline, and text in Emacs that has a
'help-echo' property is usually fontified with a face inheriting from
'link' (outside of the mode line, images, and buttons, of course).

So my suggestion is to provide a new face inheriting from either
'shr-link', 'link', or 'underline'.  Personally, I would also add
':underline (:style wave)', as it reminds me of the dotted underline in
external browsers:

(defface shr-abbreviation
  '((t :inherit underline :underline (:style wave)))
  "Face for <abbr> elements.")

Thoughts?  Lars?

> Let's try to get this merged <abbr title="as soon as possible">ASAP</abbr>.
>
> From a720473ad1001d3a72f9b3f3155f5488726cabde Mon Sep 17 00:00:00 2001
> From: Nick Drozd <nicholasdrozd <at> gmail.com>
> Date: Mon, 1 Jul 2019 23:46:10 -0500
> Subject: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
>
> * lisp/net/shr.el: Add 'abbr' and 'acronym' tag handling.

Nit: I think this should list the new functions, as per usual:

* lisp/net/shr.el (shr-tag-abbr, shr-tag-acronym): New functions
handling 'abbr' and 'acronym' tags, respectively.

> * etc/NEWS: Announce change in shr behavrior.
> ---
>  etc/NEWS        |  3 +++
>  lisp/net/shr.el | 15 +++++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index abbece374a..464dae2454 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -989,6 +989,9 @@ This attribute is meant to tell screen readers to ignore a tag.
>  ---
>  *** The <code ...> tag is now handled.
>  
> +---
> +*** The <abbr ...> and <acronym ...> tags are now handled.
> +

I suggest announcing all new tags (i.e. including <code>) in a single
NEWS entry.

>  ** Htmlfontify
>  
>  *** The functions 'hfy-color', 'hfy-color-vals' and
> diff --git a/lisp/net/shr.el b/lisp/net/shr.el
> index 7fdb3212d4..3af3db637a 100644
> --- a/lisp/net/shr.el
> +++ b/lisp/net/shr.el
> @@ -1470,6 +1470,21 @@ shr-tag-a
>      (when url
>        (shr-urlify (or shr-start start) (shr-expand-url url) title))))
>  
> +(defun shr-tag-abbr (dom)
> +  (let ((title (dom-attr dom 'title))
> +	(start (point)))
> +    (shr-generic dom)
> +    (shr-add-font start (point) 'diary)
> +    (add-text-properties
> +     start (point)
> +     (list
> +      'help-echo title
> +      'mouse-face 'highlight))))

According to Mozilla[1], the 'title' attribute is optional, so perhaps
we should avoid this fontification if the attribute is missing, which is
what Firefox and Chromium seem do.

[1]: https://developer.mozilla.org/en/docs/Web/HTML/Element/abbr

Apart from these minor comments, LGTM.

Thanks,

-- 
Basil

Message #25 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 36475 <at> debbugs.gnu.org, Nicholas Drozd <nicholasdrozd <at> gmail.com>
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Fri, 05 Jul 2019 15:03:39 +0200

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

> (defface shr-abbreviation
>   '((t :inherit underline :underline (:style wave)))
>   "Face for <abbr> elements.")
>
> Thoughts?  Lars?

Sounds good.

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

Message #28 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: Nicholas Drozd <nicholasdrozd <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 36475 <at> debbugs.gnu.org
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Fri, 5 Jul 2019 12:36:04 -0400

[Message part 1 (text/plain, inline)]

Thanks for the comments, Basil. Attached is a revised patch that
covers everything <abbr title="as far as I can tell">AFAICT</abbr>.

[0001-Handle-abbr-and-acronym-tags-in-shr.el.patch (text/x-patch, attachment)]

Message #31 received at 36475 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Nicholas Drozd <nicholasdrozd <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 36475 <at> debbugs.gnu.org
Subject: Re: bug#36475: [PATCH] Handle 'abbr' and 'acronym' tags in shr.el
Date: Sat, 06 Jul 2019 14:02:58 +0200

Nicholas Drozd <nicholasdrozd <at> gmail.com> writes:

> Thanks for the comments, Basil. Attached is a revised patch that
> covers everything <abbr title="as far as I can tell">AFAICT</abbr>.

Looks good; I've now applied this to trunk.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.