Closed Bug 1368738 Opened 4 years ago Closed 4 years ago

Anchor links in mail not working

Categories

(MailNews Core :: Security, defect)

defect
Not set
normal

Tracking

(thunderbird_esr5254+ fixed, thunderbird54 fixed, thunderbird55 fixed)

VERIFIED FIXED
Thunderbird 55.0
Tracking Status
thunderbird_esr52 54+ fixed
thunderbird54 --- fixed
thunderbird55 --- fixed

People

(Reporter: Peter_DiCamillo, Assigned: jorgk-bmo)

References

Details

(Keywords: regression)

Attachments

(2 files)

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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: Untriaged → Security
Product: Thunderbird → MailNews Core
Version: 54 Branch → 54
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.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8872763 - Flags: review?(acelists)
Yon can apply this, too, to see what's going on.
Some more explanation: Bug 1347598 introduced that "principal spec", but bug 1361020 started using it in the content policy check.
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)?
Attachment #8872763 - Flags: review?(acelists) → review+
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
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
Comment on attachment 8872763 [details] [diff] [review]
1368738-ignore-ref.patch (v1).

[Approval Request Comment]
Regression caused by (bug #): bug 1361020
Attachment #8872763 - Flags: approval-comm-esr52?
Attachment #8872763 - Flags: approval-comm-beta+
55.0a1
Build ID 	20170531030205
Status: RESOLVED → VERIFIED
Attachment #8872763 - Flags: approval-comm-esr52? → approval-comm-esr52+
You need to log in before you can comment on or make changes to this bug.