882977 - Identity state of IDENTITY_MODE_UNKNOWN is set for about:home when a new window is created

Status: RESOLVED FIXED

(Firefox :: Security, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

about:home should have an identity state of IDENTITY_MODE_CHROMEUI, but instead it ends up with an identity state of IDENTITY_MODE_UNKNOWN when a new window is created.

Setting a breakpoint within onSecurityChange shows that the location is set as about:blank, which leads the location.protocol to become "http:".

Gavin, do you know how we can change gBrowser.contentWindow.location to return about:home when a new window is created instead of about:blank?

Comment 1

[:Gavin Sharp [email: [e-mail]]]

(In reply to Jared Wein [:jaws] from comment #0)
> about:home should have an identity state of IDENTITY_MODE_CHROMEUI, but
> instead it ends up with an identity state of IDENTITY_MODE_UNKNOWN when a
> new window is created.

What do you mean by "when a new window is created"? Are you saying there's a behavior difference between loading about:home in an existing window, and having it load in a new window (with e.g. Shift+Go button)? STR would help.

From what you describe, it seems likely that we'd need to change when onSecurityChange is firing (frequency or timing).

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #1)
> (In reply to Jared Wein [:jaws] from comment #0)
> > about:home should have an identity state of IDENTITY_MODE_CHROMEUI, but
> > instead it ends up with an identity state of IDENTITY_MODE_UNKNOWN when a
> > new window is created.
> 
> What do you mean by "when a new window is created"? Are you saying there's a
> behavior difference between loading about:home in an existing window, and
> having it load in a new window (with e.g. Shift+Go button)? STR would help.
> 
> From what you describe, it seems likely that we'd need to change when
> onSecurityChange is firing (frequency or timing).

Sorry, here's clearer STR:

1) In a pre-existing window, visit about:home
2) Use DOMi to check the className on #identity-box
3) It should be '.chromeUI'
4) Make sure that your home page is set to about:home
5) Press Ctrl+N (or Cmd+N if on a Mac) to open a new window
6) Use DOMi to check the className on #identity-box

Expected:
7a) It should be '.chromeUI'

Actual:
7b) It is '.unknownIdentity'

Comment 3

[Valerio]

(In reply to Jared Wein [:jaws] from comment #2)
> (In reply to :Gavin Sharp (use [e-mail] for email) from comment
> #1)
> > (In reply to Jared Wein [:jaws] from comment #0)
> > > about:home should have an identity state of IDENTITY_MODE_CHROMEUI, but
> > > instead it ends up with an identity state of IDENTITY_MODE_UNKNOWN when a
> > > new window is created.
> > 
> > What do you mean by "when a new window is created"? Are you saying there's a
> > behavior difference between loading about:home in an existing window, and
> > having it load in a new window (with e.g. Shift+Go button)? STR would help.
> > 
> > From what you describe, it seems likely that we'd need to change when
> > onSecurityChange is firing (frequency or timing).
> 
> Sorry, here's clearer STR:
> 
> 1) In a pre-existing window, visit about:home
> 2) Use DOMi to check the className on #identity-box
> 3) It should be '.chromeUI'
> 4) Make sure that your home page is set to about:home
> 5) Press Ctrl+N (or Cmd+N if on a Mac) to open a new window
> 6) Use DOMi to check the className on #identity-box
> 
> Expected:
> 7a) It should be '.chromeUI'
> 
> Actual:
> 7b) It is '.unknownIdentity

In Nightly another similar problem is that, for example, if I select from the help menu "Troubleshooting information" the page that opens have '.unknownIdentity' and shows the globe in the identity box but if I reload the page with the stop and reload button the identity state of the page become '.chromeUI' and shows the firefox icon in the identity box

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: browser-chrome test that reproduces the issue

Tim, what do you think about landing this test now?

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 772124 [details] [diff] [review]
browser-chrome test that reproduces the issue

>+  gNewWin = openDialog(getBrowserURL(), "_blank",
>+                          "chrome,dialog=no,location=yes,toolbar=yes", "about:home");

OpenBrowserWindow()

>+  ok(gNewWin, "got new window");
>+  whenDelayedStartupFinished(gNewWin, function() {
>+    let firstTab = gNewWin.gBrowser.selectedTab;
>+
>+    // This is being tracked by bug 882977.
>+    todo_is(getIdentityMode(), "chromeUI", "Identity should be chrome");

Delayed startup merely starts loading about:home. What makes you think the identity mode should already be updated by the time this check happens?

>+    is(getIdentityMode(), "unknownIdentity", "Identity should be chrome but is currently " +
>+                                             "shown as unknownIdentity for new windows.");

Given the above todo_is, what's the point of this?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Dão Gottwald [:dao] from comment #6)
> >+    is(getIdentityMode(), "unknownIdentity", "Identity should be chrome but is currently " +
> >+                                             "shown as unknownIdentity for new windows.");
> 
> Given the above todo_is, what's the point of this?

Because while todo_is checks that the value isn't passing, it doesn't verify that the value isn't changing between test runs. If the test starts showing that about:home is having an identity mode of something other than unknownIdentity, while not chromeUI, then that would be a worthwhile issue to note.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Jared Wein [:jaws] from comment #7)
> (In reply to Dão Gottwald [:dao] from comment #6)
> > >+    is(getIdentityMode(), "unknownIdentity", "Identity should be chrome but is currently " +
> > >+                                             "shown as unknownIdentity for new windows.");
> > 
> > Given the above todo_is, what's the point of this?
> 
> Because while todo_is checks that the value isn't passing, it doesn't verify
> that the value isn't changing between test runs. If the test starts showing
> that about:home is having an identity mode of something other than
> unknownIdentity, while not chromeUI, then that would be a worthwhile issue
> to note.

But I see now that with the is() check, the todo_is() is no longer necessary.

Comment 9

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: browser-chrome test that reproduces the issue v.2

Addressed feedback.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 772130 [details] [diff] [review]
browser-chrome test that reproduces the issue v.2

>+  whenDelayedStartupFinished(win, function() {
>+    let browser = win.gBrowser.selectedBrowser;
>+    if (browser.contentDocument.readyState === "complete") {
>+      checkIdentityMode(win);
>+      return;
>+    }

The strict type checking of === isn't useful here, please use the way more common ==.

Are you sure this isn't racing anymore? When starting to load something, document.readyState is still "complete" for the previously loaded document (e.g. about:blank).

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Dão Gottwald [:dao] from comment #10)
> Comment on attachment 772130 [details] [diff] [review]
> browser-chrome test that reproduces the issue v.2
> 
> >+  whenDelayedStartupFinished(win, function() {
> >+    let browser = win.gBrowser.selectedBrowser;
> >+    if (browser.contentDocument.readyState === "complete") {
> >+      checkIdentityMode(win);
> >+      return;
> >+    }
> 
> The strict type checking of === isn't useful here, please use the way more
> common ==.

I brought this over from http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/head.js#216.

Comment 12

[Dão Gottwald [:dao]]

Yes, there's existing code where === is used for no specific reason, but == is still more common.

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: browser-chrome test that reproduces the issue v.3

Comment 14

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: browser-chrome test that reproduces the issue v.3.1

Forgot to undo a debug change.

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 772380 [details] [diff] [review]
browser-chrome test that reproduces the issue v.3.1

>+  registerCleanupFunction(function() {
>+    Services.prefs.clearUserPref("browser.startup.homepage");
>+    Services.prefs.clearUserPref("browser.startup.page");
>+  });

move win.close() here?

Comment 16

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Backed out for perma-oranging "Bug 885965 - Intermittent browser_typeAheadFind.js | Test timed out | Found a browser window after previous test timed out".

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

Comment 18

[Jared Wein [:jaws] (please needinfo? me)]

Landed on top of a fix for bug 885965.

https://hg.mozilla.org/integration/mozilla-inbound/rev/36e932df9f6e

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/36e932df9f6e