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

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[(no longer active)]

Attachment: 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.

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Regressed on early August. m-c regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5b54831761b1&tochange=f3b757156f69

continuing mozregression on inbound..

Comment 2

[:Felipe Gomes (needinfo for replies!)]

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).

Comment 3

[:Felipe Gomes (needinfo for replies!)]

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.

Comment 4

[(no longer active)]

Interesting!  Seems plausible.

Comment 5

[:Felipe Gomes (needinfo for replies!)]

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.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Attachment: MozReview Request: Bug 1196973 - Update securityUI state when changing remoteness. r?Mossop

Bug 1196973 - Update securityUI state when changing remoteness. r?Mossop

Comment 7

[:Felipe Gomes (needinfo for replies!)]

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.

Comment 8

[:Felipe Gomes (needinfo for replies!)]

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

Comment 9

[Dave Townsend [:mossop]]

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.

Comment 10

[:Felipe Gomes (needinfo for replies!)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3671af780116

Comment 11

[:Felipe Gomes (needinfo for replies!)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a95f8756b2b

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/ac8bf0e46b5a

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ac8bf0e46b5a