Closed Bug 1368738 Opened 4 years ago Closed 4 years ago
Anchor links in mail not working
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
Comment on attachment 8872763 [details] [diff] [review] 1368738-ignore-ref.patch (v1). [Approval Request Comment] Regression caused by (bug #): bug 1361020
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.