Fix broken default theme check for the early blank window in BrowserGlue.js

RESOLVED FIXED in Firefox 68

Status

()

defect
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: dao, Assigned: myk)

Tracking

(Regression, {regression})

68 Branch
mozilla68
Desktop
All
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 unaffected, firefox68+ fixed)

Details

Attachments

(2 attachments)

Reporter

Description

3 months ago

See https://phabricator.services.mozilla.com/D25678#inline-149937

Apparently that comment wasn't addressed before landing.

Component: General → Add-ons Manager
Flags: needinfo?(aswan)
Product: Firefox → Toolkit

Updated

3 months ago
Flags: needinfo?(aswan) → needinfo?(kmaglione+bmo)

(In reply to Dão Gottwald [::dao] from comment #0)

See https://phabricator.services.mozilla.com/D25678#inline-149937

Apparently that comment wasn't addressed before landing.

I replied to your comment. I don't see the issue. If you can explain it more clearly, perhaps I can do something about it.

Flags: needinfo?(kmaglione+bmo)
Reporter

Comment 2

3 months ago

(In reply to Kris Maglione [:kmag] from comment #1)

(In reply to Dão Gottwald [::dao] from comment #0)

See https://phabricator.services.mozilla.com/D25678#inline-149937

Apparently that comment wasn't addressed before landing.

I replied to your comment.

I don't see it. Is your comment submitted?

I don't see the issue. If you can explain it more clearly, perhaps I can do something about it.

This:

if (Services.prefs.getCharPref("extensions.activeThemeID", "") !=
      "default-theme@mozilla.org")

is supposed to detect non-default themes. As far as I can tell, extensions.activeThemeID isn't set by default in all.js or firefox.js, and so when the user never selected a theme, the second argument to getCharPref, the empty string, will be used as a fallback, and that's not going to match "default-theme@mozilla.org".

Flags: needinfo?(kmaglione+bmo)
Reporter

Comment 3

3 months ago

[Tracking Requested - why for this release]: This likely regresses bug 1485629.

Version: Trunk → 68 Branch

Comment 5

2 months ago
Pushed by myk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a5852f84bd26
Fix broken default theme check for early blank window. r=kmag,dao

Comment 6

2 months ago

Backed out for perma fails on browser/base/content/test/performance/browser_startup.js

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&classifiedState=unclassified&searchStr=linux%2Cshippable%2Copt%2Cmochitests%2Cwith%2Ce10s%2Ctest-linux32-shippable%2Fopt-mochitest-browser-chrome-e10s-7%2Cm-e10s%28bc7%29&revision=a5852f84bd26624f14ada6e8e5217d471309e782&selectedJob=240200371

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=240200371&repo=autoland&lineNumber=2587

[task 2019-04-14T01:03:24.159Z] 01:03:24 INFO - TEST-START | browser/base/content/test/performance/browser_startup.js
[task 2019-04-14T01:03:25.776Z] 01:03:25 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_startup.js | should have no unexpected modules loaded before profile selection - Got 1, expected 0
[task 2019-04-14T01:03:25.777Z] 01:03:25 INFO - Stack trace:
[task 2019-04-14T01:03:25.778Z] 01:03:25 INFO - chrome://mochikit/content/browser-test.js:test_is:1325
[task 2019-04-14T01:03:25.778Z] 01:03:25 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_startup.js:null:201
[task 2019-04-14T01:03:25.779Z] 01:03:25 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1116
[task 2019-04-14T01:03:25.780Z] 01:03:25 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1144
[task 2019-04-14T01:03:25.781Z] 01:03:25 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1005
[task 2019-04-14T01:03:25.782Z] 01:03:25 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-04-14T01:03:25.783Z] 01:03:25 INFO - Not taking screenshot here: see the one that was previously logged

Backout: https://hg.mozilla.org/integration/autoland/rev/2861cc36215463fc67cbd217c3eb7f3a62115097

Flags: needinfo?(myk)
Reporter

Updated

2 months ago
Assignee: nobody → myk
Assignee

Comment 8

2 months ago

In bug 1541798, comment 7, dao wrote:

You probably need to revert and update this: https://hg.mozilla.org/mozilla-central/diff/83612982ab33/browser/base/content/test/performance/browser_startup.js

Ah, right, this version reverts and updates that code.

Here's a tryserver run for the test group that failed the last time around:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a36b6de0c6007705062219cc6309bc14c1bc369

Comment 9

2 months ago
Pushed by myk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d2077420c62
Fix broken default theme check for early blank window. r=dao
Assignee

Comment 10

2 months ago

I fixed the test failure in the first patch and autolanded a new version of the patch.

Flags: needinfo?(myk)
Flags: needinfo?(kmaglione+bmo)

Comment 11

2 months ago
bugherder
Status: NEW → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Updated

2 months ago
Regressions: 1545062
See Also: → 1546140

Updated

2 months ago
Regressions: 1543152
No longer regressions: 1543152, 1545062
You need to log in before you can comment on or make changes to this bug.