Here's a stack: [task 2017-08-01T16:03:50.580286Z] 16:03:50 INFO - Console message: [JavaScript Error: "TypeError: this.mainViewNode is null" {file: "chrome://browser/content/browser-pageActions.js" line: 43}] [task 2017-08-01T16:03:50.583316Z] 16:03:50 INFO - get mainViewBodyNode@chrome://browser/content/browser-pageActions.js:43:5 [task 2017-08-01T16:03:50.586298Z] 16:03:50 INFO - placeActionInPanel@chrome://browser/content/browser-pageActions.js:98:7 [task 2017-08-01T16:03:50.590435Z] 16:03:50 INFO - placeAction@chrome://browser/content/browser-pageActions.js:72:5 [task 2017-08-01T16:03:50.592522Z] 16:03:50 INFO - init@chrome://browser/content/browser-pageActions.js:51:7 [task 2017-08-01T16:03:50.594536Z] 16:03:50 INFO - onLoad@chrome://browser/content/browser.js:1393:5 [task 2017-08-01T16:03:50.596625Z] 16:03:50 INFO - onload@chrome://browser/content/browser.xul:1:1 [task 2017-08-01T16:03:50.598555Z] 16:03:50 INFO - [task 2017-08-01T16:03:50.602123Z] 16:03:50 INFO - Buffered messages logged at 16:02:19 [task 2017-08-01T16:03:50.604483Z] 16:03:50 INFO - Longer timeout required, waiting longer... Remaining timeouts: 1 [task 2017-08-01T16:03:50.606428Z] 16:03:50 INFO - Buffered messages finished [task 2017-08-01T16:03:50.608860Z] 16:03:50 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/captivePortal/browser_CaptivePortalWatcher.js | Test timed out - I suspect that this is because we're initting the BrowserPageActions object regardless of whether or not Photon is enabled. We should probably put the initialization of BrowserPageActions behind either a build-time or run-time conditional.

For sheriffs - if this doesn't make the merge in time, I'm pretty sure bug 1374477 can be backed out on the busted beta and that'll fix this.

This should do it. Doing a run-time check seems to be what other browser-* scripts do. This made me realize that my fix to the Pocket bug 1367927 assumed that Photon is enabled. So that will probably break Cedar too? (If not, it probably will once this patch here lands, since the patch bails out of PageActions.init, which will probably screw up Pockets' PageActions.addAction call, but I'm not sure.) Gijs, what do you think the strategy should be for PageActions consumers to determine whether page actions are available? I'm thinking they should check AppConstants.MOZ_PHOTON_THEME before attempting to use PageActions.jsm. If they don't, then they'll break, or at least their actions won't show up. Alternatively we could just not bundle PageActions.jsm at build-time. That way consumers would at least get an error when trying to load the jsm. But Photon not being enabled is just a temporary thing, right? So it's not worth coming up with the consumer-friendliest solution IMO.

I filed bug 1386474 for Pocket and use AppConstants.MOZ_PHOTON_THEME, which makes that bug very simple.

The browser_PageActions.js test needs to run on Nightly only. (There doesn't seem to be a Photon run-if/skip-if?) The other two page actions-related tests, browser_page_action_menu.js and browser_page_action_menu_clipboard.js, already have a `run-if nightly_build`.

I'm trying to push this patch to try with Photon configure'ed off. I screwed it up with the last try push, looks like. Trying again with all "photon" things defaulted to false. Is there some other way to test this on Cedar? Other than landing it.

(In reply to Drew Willcoxon :adw from comment #9) > I'm trying to push this patch to try with Photon configure'ed off. I > screwed it up with the last try push, looks like. Trying again with all > "photon" things defaulted to false. > > Is there some other way to test this on Cedar? Other than landing it. You can push Cedar to try.

(In reply to Mike Conley (:mconley) - Buried in needinfo / review backlog, eating my way out from comment #10) > You can push Cedar to try. (I happen to have a Cedar tree cloned locally - if you'd rather I push it to try for you, needinfo me, and I'll do it when I wake up tomorrow)

Ah yeah, thanks Mike. I'll try that.

Just realized the bookmark star button is ifdef'ed out of browser.xul when building without Photon.

That's a Cedar push to try with the patch. Not sure whether comment 13 is right/relevant. Does CUI include the bookmark menu panel button when Photon is not enabled? Guess I'll find out.

Comment on attachment 8892710 [details] Bug 1386337 - Disable Photon page actions when Photon is not enabled. https://reviewboard.mozilla.org/r/163700/#review169126 rs=me. Note that this doesn't fix the browser_pageActions.js test on the trypush or, I expect, on cedar, but it'll work on beta.

Triggered autoland, but bug 1386528 is keeping the tree closed.

Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/84230e88dfef Disable Photon page actions when Photon is not enabled. r=Gijs

[Tracking Requested - why for this release]: This puts some Photon-specific things (the Page Actions code) behind some conditionals so that it doesn't run (and throw exceptions) when various DOM nodes it wants to work with aren't there. Without this, the browser load event handler will, I believe, throw an exception, and a bunch of stuff won't get enabled and the browser will be pretty busted.

Comment on attachment 8892710 [details] Bug 1386337 - Disable Photon page actions when Photon is not enabled. Approval Request Comment [Feature/Bug causing the regression]: Photon stuff that was working on Nightly, but is disabled / not built on Beta. Specifically, bug 1374477. [User impact if declined]: Probably loads of stuff in the browser just won't work - at least, according to the oranges on beta. [Is this code covered by automated tests?]: It sure is, that's why beta is currently as orange as a carrot. [Has the fix been verified in Nightly?]: adw pushed to try on Cedar (which is the branch that simulates Photon being disabled) and it came back looking good. [Needs manual test from QE? If yes, steps to reproduce]: None. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: We're just making sure that some Photon stuff is a no-op on 56, which doesn't ship with Photon-y things. [String changes made/needed]: None.

Track 56+ as test failures on mozilla-beta.

Comment on attachment 8892710 [details] Bug 1386337 - Disable Photon page actions when Photon is not enabled. Anti-carrot squad, please uplift to beta.

(In reply to Mike Conley (:mconley) - Holiday until August 8th from comment #20) > [Is this code covered by automated tests?]: > > It sure is, that's why beta is currently as orange as a carrot. > > [Has the fix been verified in Nightly?]: > > adw pushed to try on Cedar (which is the branch that simulates Photon being > disabled) and it came back looking good. > > [Needs manual test from QE? If yes, steps to reproduce]: > > None. Setting qe-verify- based on Mike's assessment on manual testing needs and the fact that this fix has automated coverage.