1971035 - open link uses double coding of links in RSS feeds when clicking the Website header link

Status: RESOLVED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Thomas]

Attachment: RSS-feed1.jpg

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:139.0) Gecko/20100101 Firefox/139.0

Steps to reproduce:

I have subscribed to the RSS feed below. Then I clicked on the link displayed under the "Website" line of any article:
https://www.floersheim-main.de/media/rss/MitteilungenderStadtFloersheimamMain_1.xml

Actual results:

The article can not be reed because the Website does not show the text.

Firefox version 139.01 has been opened. The opened web site does not showing the article because the link has been changed (duble encoding) after clicking on the Link.
Example:

Original Link:
https://www.floersheim-main.de/index.php?ModID=255&FID=2983.37625.1&object=tx%2C2983.5

Chaged Link:https://www.floersheim-main.de/index.php?ModID=255&FID=2983.37625.1&object=tx%252C2983.5

The part of the link containing "%2C" has been changed automaticaly to "%252C".

Expected results:

The correct website schoul be shown. The link should not be double-encoded.

Comment 1

[Magnus Melin [:mkmelin]]

Confirming.
Seems https://searchfox.org/comm-central/rev/2a834ea2a5a5f741da3d2272850bbfbb82b232bc/mail/base/content/widgets/header-fields.js#727,732 should not encode.

Comment 2

[Nazar Mokrynskyi]

Similar issue is happening with links to +security- releases in GitHub releases here: https://github.com/grafana/grafana/releases.atom
+ is not encoded properly, leading to the 404 page.

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: Bug 1971035 - Opening Website header link should not re-encode parameters in the URL before opening it. r=darktrojan

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/388915d6a86b
Opening Website header link should not re-encode parameters in the URL before opening it. r=darktrojan

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9495730 [details]
Bug 1971035 - Opening Website header link should not re-encode parameters in the URL before opening it. r=darktrojan

Uplift Approval Request

• Please state case for uplift consideration and ensure bug severity is set: Broken functionality
• User impact if declined: Opening feed links may not work for specific linke
• Is this code covered by automated tests?: No
• Has the fix been verified in Daily?: Yes
• Has the fix been verified in Beta?: Yes
• Needs manual test from QA?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Confined change
• Does the fix cause any migrations to be skipped?: No
• String changes made/needed: none

Comment 6

[Corey Bryant]

Comment on attachment 9495730 [details]
Bug 1971035 - Opening Website header link should not re-encode parameters in the URL before opening it. r=darktrojan

[Triage Comment]
Approved for esr140

Comment 7

[Kai Engert [:KaiE:]]

The backport didn't work.

TEST-UNEXPECTED-FAIL | comm/mailnews/extensions/newsblog/test/browser/browser_feedDisplay.js | Uncaught exception in test bound testRSS - at chrome://mochitests/content/browser/comm/mailnews/extensions/newsblog/test/browser/browser_feedDisplay.js:221 - ReferenceError: MockExternalProtocolService is not defined

https://treeherder.mozilla.org/jobs?repo=comm-esr140&selectedTaskRun=XkViD_aoR9qRrmEItOFmew.0

Comment 8

[Magnus Melin [:mkmelin]]

Mentioned it elsewhere but that needs to be lowercase mockExternalProtocolService on 140

Comment 9

[Kai Engert [:KaiE:]]

(In reply to Magnus Melin [:mkmelin] from comment #8)

Mentioned it elsewhere but that needs to be lowercase mockExternalProtocolService on 140

I cannot find assertHasLoadedURL() on 140.

Comment 10

[Magnus Melin [:mkmelin]]

Oh I see, then it depends on bug 1972727.
It would probably be best then to just comment out the 5 test lines added (for 140) - https://hg-edge.mozilla.org/comm-central/rev/388915d6a86b#l2.12

Comment 11

[Kai Engert [:KaiE:]]

Magnus, I suggest to undo the "fixed 140" flag, because it isn't right?

Would you like to prepare a patch with the changes you're suggesting?

Comment 12

[Magnus Melin [:mkmelin]]

It still says 140 affected, no?

I don't have a 140 tree checked out atm so if you want to do it, feel free to.

Comment 13

[Kai Engert [:KaiE:]]

You're right, it isn't marked as fixed yet.

However, the patch has already landed into esr140 !

https://hg-edge.mozilla.org/releases/comm-esr140/rev/8dc31fbdf16624a4c39c0fc99a4e0ad3060b3ee0

Comment 14

[Daniel Darnell [:dandarnell]]

This was not marked as fixed as I'm still not sure if it'll need to be backed out (QA has not tested 140.3.0esr yet). The patch caused a permafail on the promoted build we're about to test.

Comment 15

[Magnus Melin [:mkmelin]]

Attachment: Bug 1971035 - [comm-esr140] - Drop test changes from 140 port. r=dandarnell

Drop test changes from 140, as they are not compatbile (even if the code change is).

Comment 16

[Magnus Melin [:mkmelin]]

I think that's a confusing workflow. What I'd do:

• apply all you think you want for esr locally
• run linters locally, see if there are obvious issues (would have caught this)
• push the stack to try
• if the try run looks decent, push the local stack to comm-esr140
• as soon as landed, mark the bug fixed and have the commit show (this should be automated, IMHO)
• if there are issues, circle back and figure out why (and mark appropriately)

Comment 17

[Magnus Melin [:mkmelin]]

Daniel: but it did ship in 140.3.0, and there's still no marks in this bug. Please add the commit, to this (and others?) where it landed.
Of course, the 140esr tree is not overly happy so the patch above should still be landed.

Comment 18

[Corey Bryant]

Comment on attachment 9513670 [details]
Bug 1971035 - [comm-esr140] - Drop test changes from 140 port. r=dandarnell

[Triage Comment]
Approved for esr140

Comment 19

[Daniel Darnell [:dandarnell]]

Thunderbird 140.3.0:
https://hg.mozilla.org/releases/comm-esr140/rev/8dc31fbdf166

Comment 20

[Daniel Darnell [:dandarnell]]

I'm leaving this marked as "affected" until the follow-up patch is uplifted, so that the bug will still show up in our uplift queue.

Comment 21

[Stephen King (:sking)]

Thunderbird 140.3.1:
https://hg.mozilla.org/releases/comm-esr140/rev/0fa6fc41927b5c25a06da8b6a70fa3fc4628e20c