Closed Bug 1338340 Opened 3 years ago Closed 3 years ago

Intermittent browser_bug435035.js | identity box has class name for mixed content - Got unknownIdentity, expected unknownIdentity mixedDisplayContent

Categories

(Firefox :: Site Identity, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: intermittent-bug-filer, Assigned: johannh)

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell fixed])

Attachments

(1 file)

Priority: -- → P3
This started failing shortly after the test was moved in bug 1335431. It was previously in browser/base/content/test/general/, but I don't see any sign of it failing in that location. 

Failures are exclusively osx-opt and e10s.
(In reply to Geoff Brown [:gbrown] from comment #3)
> This started failing shortly after the test was moved in bug 1335431. It was

Sorry, that's not true. https://treeherder.mozilla.org/logviewer.html#?repo=autoland&job_id=76069565&lineNumber=2193 and a few other were before the move.
more failures in the last week, more debug failures mixed in, but all OSX.

:johann, I see you as the triage owner for this component, if you could find someone to look at this in the next week or two that would be great.  I will keep an eye on this and make sure to note if the frequency drops down or the pattern changes.
Flags: needinfo?(jhofmann)
Whiteboard: [stockwell needswork]
Hm, the failure screenshots show that the page is not fully loaded (yet?) and many show the e10s spinner. The test seems to correctly wait for page load with BrowserTestUtils.browserLoaded, so that function might get called a little early. I'm really not sure why though, so I'm flagging mconley because he tends to be interested in this stuff.
Flags: needinfo?(jhofmann) → needinfo?(mconley)
this has reduced in frequency a bit, still showing up often
Whiteboard: [stockwell needswork] → [stockwell unknown]
A short-term fix would be to modify the usage of browserLoaded to ensure that we wait for the TEST_URL to be loaded: http://searchfox.org/mozilla-central/source/testing/mochitest/BrowserTestUtils/BrowserTestUtils.jsm#177-181
Flags: needinfo?(mconley)
Summary: Intermittent browser/base/content/test/siteIdentity/browser_bug435035.js | identity box has class name for mixed content - Got unknownIdentity, expected unknownIdentity mixedDisplayContent → Intermittent browser_bug435035.js | identity box has class name for mixed content - Got unknownIdentity, expected unknownIdentity mixedDisplayContent
this really picked up on May 24th and is on track to be the #1 failure.  I am doing some retriggers:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=osx%20debug%20browser-chrome%20e10s&tochange=6a40d59b6395f7ca26f9b17c3b013fb9ac2e8c42&fromchange=b67b7ae74a45ada64960246c27539a874e22976c&selectedJob=101352230

it might take a day or so to get these retriggers run.  In the meantime, :johannh, can you take a look at this give this high daily failure rate?
Flags: needinfo?(jhofmann)
Whiteboard: [stockwell unknown] → [stockwell needswork]
This test is really absurdly small to cause such a fallout. The BTU.browserLoaded might be picking up the initial load from addTab, is our tab loading getting slower on OSX?

I'll just modernize this test and hope that fixes it. Worst case is just waiting for the exact url to load as Mike mentioned or we just disable the thing on OSX, it's really nothing we should waste a lot of time on. I bet this condition is checked in half a dozen other tests.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Flags: needinfo?(jhofmann)
Comment on attachment 8872353 [details]
Bug 1338340 - Modernize browser_bug435035.js.

https://reviewboard.mozilla.org/r/143836/#review147632

I'd expect this to fix the failure, thanks! One minor comment (since we're modernizing anyway).

::: browser/base/content/test/siteIdentity/browser_mixed_passive_content_indicator.js:6
(Diff revision 1)
> -  waitForExplicitFinish();
> +  await BrowserTestUtils.withNewTab(TEST_URL, function() {
> -
> -  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
> -  BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser).then(() => {
>      is(document.getElementById("identity-box").className,
>         "unknownIdentity mixedDisplayContent",

If we only care about the mixedDisplayContent class, we should use classList.contains(), no?
Attachment #8872353 - Flags: review?(nhnt11) → review+
Comment on attachment 8872353 [details]
Bug 1338340 - Modernize browser_bug435035.js.

https://reviewboard.mozilla.org/r/143836/#review147764

::: browser/base/content/test/siteIdentity/browser_mixed_passive_content_indicator.js:6
(Diff revision 1)
> -  waitForExplicitFinish();
> +  await BrowserTestUtils.withNewTab(TEST_URL, function() {
> -
> -  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
> -  BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser).then(() => {
>      is(document.getElementById("identity-box").className,
>         "unknownIdentity mixedDisplayContent",

Ah, indeed. Very good observation, thanks!
Comment on attachment 8872353 [details]
Bug 1338340 - Modernize browser_bug435035.js.

https://reviewboard.mozilla.org/r/143836/#review147764

> Ah, indeed. Very good observation, thanks!

On the other hand, looking at the CSS rules in https://searchfox.org/mozilla-central/source/browser/themes/shared/identity-block/identity-block.inc.css#199, there are a number of classnames that could override the mixed display icon. We want to be sure the correct icon is shown, so let's keep it like that.
Keywords: leave-open
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b0382a33f8c4
Modernize browser_bug435035.js. r=nhnt11
Whiteboard: [stockwell needswork] → [stockwell fixed]
Looks fixed to me!
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.