What's new link in the "Firefox Updates" section of the preferences is broken (opens blank page)

VERIFIED FIXED in Firefox 65

Status

()

defect
P1
normal
VERIFIED FIXED
5 months ago
4 months ago

People

(Reporter: agashlin, Assigned: baku)

Tracking

({regression})

unspecified
Firefox 66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox63 unaffected, firefox64 unaffected, firefox65+ verified, firefox66 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 months ago
Clicking the "What's new" link in Nightly Updates opens a new blank tab. The address bar is empty but it seems to be about:blank. This seems to have started with bug 1503681 changing the behavior of target=_blank.

Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b526d511169a3ba8ea647d6de9183a62df0da1c8&tochange=e6a4330eb15f1536f505d6794c4efac3ca9fcf7e

Comment 1

5 months ago
[Tracking Requested - why for this release]:
Broken functionality that regressed recently.

:baku, can you take a look?

To be clear, this is the "What's new" link in the "Nightly Updates" section in the prefs (find easily by just sticking 'update' in the filter in the options/preferences). The one in the "About Nightly/Firefox" window works fine.

There don't seem to be any errors in the browser console.
Flags: needinfo?(amarchesini)
Keywords: regression
Priority: -- → P1
Summary: What's new link opens about:blank → What's new link in the "Firefox Updates" section of the preferences is broken (opens blank page)
(Assignee)

Comment 2

5 months ago
Posted patch rel.patch (obsolete) — Splinter Review
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #9029862 - Flags: review?(gijskruitbosch+bugs)

Comment 3

5 months ago
Comment on attachment 9029862 [details] [diff] [review]
rel.patch

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

Does rel=opener normally imply that the opened page has access to the opener window via window.opener? In that case, it'd mean the opened page can access the system privileged about:preferences . That seems like a poor idea. It doesn't seem to do that right now, but it's completely unclear why, if we can depend on that continuing to be the case -- and more importantly, why this fix is necessary. At a minimum, we need an explanation as to why we need to do this. Why doesn't the link "just work"?
Attachment #9029862 - Flags: review?(gijskruitbosch+bugs) → review-
(Assignee)

Comment 4

5 months ago
In bug 1503681, we introduced an implicit rel=noopener for target=_blank anchor elements. What happens here is that, a privilege page has a link to a non-privilege page. As any other loading we want to be the triggering principal:

https://searchfox.org/mozilla-central/rev/adec563403271e78d1a057259b3e17fe557dfd91/docshell/base/nsDocShell.cpp#8997-8999

But when we setup the inheriting principal, we explicit check that:

  // If principalIsExplicit *is* set, there are 4 possibilities
  // (1) If the system principal or an expanded principal was passed in
  //     and we're a typeContent docshell, return an error.

https://searchfox.org/mozilla-central/rev/adec563403271e78d1a057259b3e17fe557dfd91/docshell/base/nsDocShellLoadState.cpp#266-268

To be compatible with the previous configuration, we should manually force a rel='opener'.
Alternatively, we can disable bug 1503681 when the triggering principal is system.
Nika, what do you suggest here?
Flags: needinfo?(nika)
I think the cleanest option here is to not cause noopener when our triggering principal is system, and file a follow-up to come up with a cleaner solution here.
Flags: needinfo?(nika)
(Assignee)

Comment 6

4 months ago
Posted patch rel.patchSplinter Review
Attachment #9029862 - Attachment is obsolete: true
Attachment #9030536 - Flags: review?(nika)
Attachment #9030536 - Flags: review?(nika) → review+

Comment 7

4 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd22bf7a28c5
Disable implicit rel=noopener in anchor and area elements if the triggering principal is system, r=nika

Comment 8

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fd22bf7a28c5
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
See Also: → 1512303
Please nominate this for Beta approval when you get a chance.
Flags: needinfo?(amarchesini)
Flags: qe-verify+
(Assignee)

Comment 10

4 months ago
Comment on attachment 9030536 [details] [diff] [review]
rel.patch

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1503681

User impact if declined: anchor links in about:preferences would not be working.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: See the bug description

List of other uplifts needed: none

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The patch disable bug 1503681 for system principal pages. Just a simple if()-stmt.

String changes made/needed: none
Flags: needinfo?(amarchesini)
Attachment #9030536 - Flags: approval-mozilla-beta?
Comment on attachment 9030536 [details] [diff] [review]
rel.patch

[Triage Comment]
Allows anchor links in about:preferences to work again. Approved for 65.0b5.
Attachment #9030536 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I have reproduced this bug with Nightly 65.0a1 (2018-12-05) on Windows 7, 64 Bit!
This bug's fix is verified with latest Nightly!

Build ID 	20181216095236
User Agent 	Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0
QA Whiteboard: [bugday-20181212]
I managed to reproduce the issue using an older version of Nightly (2018-12-05) on Windows 10 x64.
I verified the fix using latest Nightly 66.0a1 and beta 65.0b5 on Windows 10 x64, Ubuntu 16.04 x64 and macOS 10.13. The bug is not reproducing anymore.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.