1368738 - Anchor links in mail not working

Status: VERIFIED FIXED

(MailNews Core :: Security, defect)

Description

[Peter DiCamillo]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170518000419

Steps to reproduce:

Open mail with a table of contents that uses anchor links. Click on one of the anchor links in order to jump to that item.


Actual results:

In 53.0b2 the jump occurs as expected. However, in 54.0b2 nothing happens.


Expected results:

54.0b2 should behave like 53.0b2 and perform the jump specified by the anchor link.

Comment 1

[Jorg K (CEST = GMT+2)]

Indeed, that's broken. Thanks for reporting. I'm using attachment 8785723 [details] from bug 359183 for testing.

Most likely another regression of bug 1361020, sigh.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: 1368738-ignore-ref.patch (v1).

Aceman, you've got to help me here with a quick review.

The GetPrincipalSpec() was implemented in bug 1347598 for all protocols:
https://hg.mozilla.org/comm-central/rev/291bc010edbf0fcc00600201fef1da7865555bc3#l7.12 IMAP
https://hg.mozilla.org/comm-central/rev/291bc010edbf0fcc00600201fef1da7865555bc3#l8.30 JsAccount
https://hg.mozilla.org/comm-central/rev/291bc010edbf0fcc00600201fef1da7865555bc3#l9.12 Mailbox
https://hg.mozilla.org/comm-central/rev/291bc010edbf0fcc00600201fef1da7865555bc3#l9.12 NNTP

Foolishly I forgot to strip the reference, so now, for in the content policy code it compares
mailbox:///D:/DESKTOP/link/Mail/Local%20Folders/Mai%202?number=16#h01
with
mailbox:///D:/DESKTOP/link/Mail/Local%20Folders/Mai%202?number=16

This patch fixes it. It's impossible to get a review from Kent, so please approve.

I'll attach the my content policy patch so you can see what's going on.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: content-policy-debug-1.patch - debug patch

Yon can apply this, too, to see what's going on.

Comment 4

[Jorg K (CEST = GMT+2)]

Some more explanation: Bug 1347598 introduced that "principal spec", but bug 1361020 started using it in the content policy check.

Comment 5

[:aceman]

Comment on attachment 8872763 [details] [diff] [review]
1368738-ignore-ref.patch (v1).

Review of attachment 8872763 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, this helps on the mentioned test email with links to #anchors inside the same message.
Thanks.

::: mailnews/news/src/nsNntpUrl.cpp
@@ +307,4 @@
>    // URLs look like this:
>    // news://server:port/folder?group=ggg&key=nnn [ &part=ppp &filename=fff ].
>    // Just strip the part which will also remove the filename.
>    // Normalised spec: news://server:port/folder?group=ggg&key=nnn

Do we want to mention the possible #anchor in some of these comments (all the touched files)?

Comment 6

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/2662981ddb0984098f4e74844ac7e973ac7fab80

===

(In reply to :aceman from comment #5)
> Do we want to mention the possible #anchor in some of these comments (all
> the touched files)?
No. The nomenclature is an absolute nightmare.

Has it occurred to you that links are in fact anchors?
<a href= >. That's an anchor, not a link. In Gecko: HTMLAnchorElement.
A link is something else: Like a link to a stylesheet:
<link href="xxx.css" rel="stylesheet" type="text/css">
In Gecko: HTMLLinkElement.

Sadly, anchors are commonly called links and *references*, the #stuff, (see nsIURI.idl) are called anchors.
Every time I have to work on those, I get a crisis :-(

Also called anchor is the "target" which can be an anchor <a name="target"> or <a id="target">. Nowadays, any element with an id can be the target, so no anchor required.

So which comment should I have put: Ignore anchor? Wrong! Ignore reference, hey, that's what the function is called ;-)

Just check where the whole trouble started:
https://dxr.mozilla.org/comm-central/rev/62db49c50e60514a4178651227629a9625381cb4/mail/base/content/contentAreaClick.js#82

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8872763 [details] [diff] [review]
1368738-ignore-ref.patch (v1).

[Approval Request Comment]
Regression caused by (bug #): bug 1361020

Comment 8

[Jorg K (CEST = GMT+2)]

Beta (TB 54):
https://hg.mozilla.org/releases/comm-beta/rev/a60b2ce3bfb95c50fb5b6fc95408a55756dc4240

Comment 9

[Joe Sabash [:JoeS1]]

55.0a1
Build ID 	20170531030205

Comment 10

[Jorg K (CEST = GMT+2)]

TB 52 ESR:
https://hg.mozilla.org/releases/comm-esr52/rev/90901942e6685648969ac0c7cfd7d90309801b43