Closed Bug 1196973 Opened 7 years ago Closed 7 years ago

The security UI is not updated when going back and forth between view-source and the web

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
e10s m8+ ---
firefox43 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: Felipe)

References

Details

Attachments

(2 files)

Attached image Screenshot
STR:
1. Go to view-source:about:newtab.
2. Go to https://www.mozilla.org/en-US/
3. Click back.

Observe the resulting UI.  I'm attaching a screenshot.
Assignee: nobody → felipc
Regressed on early August. m-c regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5b54831761b1&tochange=f3b757156f69

continuing mozregression on inbound..
55:18.88 LOG: MainThread Bisector INFO No inbound data found.
55:18.88 LOG: MainThread Bisector INFO There are no build artifacts on inbound for these changesets (they are probably too old).
Worth noting that this only happens if the tab being used started as about:newtab.
The bug _doesn't_ reproduce if:

- it started as a regular tab (through e.g. cmd+clicking on a link to open as a new tab, or about:home as the first tab of a window)
- it started as another non-e10s tab like the Add-ons Manager (by pressing Cmd+Shift+A)
- it went through about:newtab (changing remoteness) but it's not the first tab in the history

So I believe this rules out a direct involvement of the remoteness switching code, and my hunch is that it's caused by the code that special cases about:newtab from not appearing as the first entry in session history.
Interesting!  Seems plausible.
Ah, more info. This doesn't reproduce for view-source:about:blank, or view-source:http://www.example.com,
but it does for view-source:about:newtab, view-source:about:addons...
So this may invalidate comment 2 and 3 because I was interchangeably using about:newtab and about:blank while testing.

But: If I open the Browser Toolbox to try to step through the debugger, some network listeners for the Network panel will catch exceptions while view-source:about:newtab is loading, and the load process never completes to the point to where I can debug (good old Heisenbug).

However, this leads to a new theory that these same exceptions are being thrown somewhere in the code that does the securityUI updates and it's causing this problem. I'll keep digging.
Bug 1196973 - Update securityUI state when changing remoteness. r?Mossop
Attachment #8657939 - Flags: review?(dtownsend)
https://reviewboard.mozilla.org/r/18433/#review16523

::: browser/base/content/tabbrowser.xml:1563
(Diff revision 1)
> +                                        true, false);

This is basically copied over from UpdateCurrentBrowser. There's a check for `if (securityUI)` that I don't why it exists (are there any cases of securityUI not existing?). Anyway, that check goes back to the original tabbrowser.xml implementation. Instead of doing the same I preferred a safer approach of calling onSecurityChange anyway if that doesn't exist, with STATE_IS_INSECURE.
So this was indeed caused by the browsing switching remoteness. It didn't happen before because view-source was allowed to open in e10s, but in bug 1175770 a condition was added [1] to make view-source follow the rules of the original URL on which process it should open (so, view-source:about:newtab will want to open in the parent process). I'm not sure why it was added but I wouldn't call that a regression from that bug, it's just exposing the existing issue.

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/0364858aed56#l14.17
Blocks: 1175770
Comment on attachment 8657939 [details]
MozReview Request: Bug 1196973 - Update securityUI state when changing remoteness. r?Mossop

https://reviewboard.mozilla.org/r/18433/#review16563

Looks good.
Attachment #8657939 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/mozilla-central/rev/ac8bf0e46b5a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
You need to log in before you can comment on or make changes to this bug.