945739 - showInPrivateBrowsing widget breaks palette construction in private window because forWindow(...).node is null

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[sean]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20131202092621

Steps to reproduce:

Updated nightly, Opened menu, then opened +customize.


Actual results:

Opens a blank page with the words "More tools to add to the menu and toolbar" Restart does not fix the problem.


Expected results:

Should have listed all the widgets and addons in the customize menu.

Comment 1

[:Gijs (he/him)]

Are there any errors in the error console? Can you reproduce this on a clean profile and/or in safe mode?

Comment 2

[sean]

(In reply to :Gijs Kruitbosch from comment #1)
> Are there any errors in the error console? Can you reproduce this on a clean
> profile and/or in safe mode?

Yep here are the errors, this is in safe mode.

console.error: 
  [CustomizeMode]
  Message: TypeError: aWindowPalette is null
  Stack:
    @resource:///modules/CustomizableUI.jsm:1204
@resource:///modules/CustomizableUI.jsm:2086
@resource://app/modules/CustomizeMode.jsm:443
Task_spawn@resource://gre/modules/Task.jsm:142
@resource://app/modules/CustomizeMode.jsm:452
@resource://app/modules/CustomizeMode.jsm:189
TaskImpl_run@resource://gre/modules/Task.jsm:233
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118
then@resource://gre/modules/commonjs/sdk/core/promise.js:43
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118
then@resource://gre/modules/commonjs/sdk/core/promise.js:43
resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185
TaskImpl_run@resource://gre/modules/Task.jsm:242
@resource://gre/modules/Promise.jsm:767
@resource://gre/modules/Promise.jsm:531

Comment 3

[:Gijs (he/him)]

(In reply to sean from comment #2)
> (In reply to :Gijs Kruitbosch from comment #1)
> > Are there any errors in the error console? Can you reproduce this on a clean
> > profile and/or in safe mode?
> 
> Yep here are the errors, this is in safe mode.
> 
> console.error: 
>   [CustomizeMode]
>   Message: TypeError: aWindowPalette is null
>   Stack:
>     @resource:///modules/CustomizableUI.jsm:1204
> @resource:///modules/CustomizableUI.jsm:2086
> @resource://app/modules/CustomizeMode.jsm:443
> Task_spawn@resource://gre/modules/Task.jsm:142
> @resource://app/modules/CustomizeMode.jsm:452
> @resource://app/modules/CustomizeMode.jsm:189
> TaskImpl_run@resource://gre/modules/Task.jsm:233
> resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118
> then@resource://gre/modules/commonjs/sdk/core/promise.js:43
> resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185
> resolve@resource://gre/modules/commonjs/sdk/core/promise.js:118
> then@resource://gre/modules/commonjs/sdk/core/promise.js:43
> resolve@resource://gre/modules/commonjs/sdk/core/promise.js:185
> TaskImpl_run@resource://gre/modules/Task.jsm:242
> @resource://gre/modules/Promise.jsm:767
> @resource://gre/modules/Promise.jsm:531

Is this the only error? Because this basically means the toolbox's palette didn't get initialized correctly. I can't reproduce this locally, though. :-(

Comment 4

[Alejandro Rodriguez [:Alopepeo]]

I can reproduce this when entering customization mode in a new private window. Here you have some screenshots: 

Customize mode: http://www.dropmocks.com/iCa8v6
Console output: http://www.dropmocks.com/iCay-7

Comment 5

[sean]

(In reply to Alejandro Rodriguez [:Alopepeo] from comment #4)
> I can reproduce this when entering customization mode in a new private
> window. Here you have some screenshots: 
> 
> Customize mode: http://www.dropmocks.com/iCa8v6
> Console output: http://www.dropmocks.com/iCay-7

Yep, your right. I can't believe I forgot to mention that.

Comment 6

[:Gijs (he/him)]

So I saw this once, tried to debug it, then promptly couldn't reproduce anymore. Can't reproduce on a clean profile, either. Really unsure what would cause this... :-\

Comment 7

[Alejandro Rodriguez [:Alopepeo]]

(In reply to :Gijs Kruitbosch from comment #6)
> So I saw this once, tried to debug it, then promptly couldn't reproduce
> anymore. Can't reproduce on a clean profile, either. Really unsure what
> would cause this... :-\
That's true, on the latest builds the problem seems to be gone, at least for now.....

Comment 8

[sean]

Yep, I just updated and the customize menu is working again. Strange. Is there a fix or commit log I can check to see if someone found and fixed it? Thanks.

Comment 9

[:Gijs (he/him)]

(In reply to sean from comment #8)
> Yep, I just updated and the customize menu is working again. Strange. Is
> there a fix or commit log I can check to see if someone found and fixed it?
> Thanks.

There is the plain hg log on https://hg.mozilla.org/mozilla-central/, but um, people commit a lot of stuff.

If you could reliably reproduce this earlier and you want to get to the bottom of this, it might be quicker to just figure out when exactly this started working again first by downloading nightlies to determine a regression range (see https://quality.mozilla.org/docs/qmo-community/lesson-plans/how-to-bisect-find-the-culprit/ or http://mozilla.github.io/mozregression/ if you're comfortable with a commandline and/or python). Then you can use the one-day range to look at the pushlog.

Comment 10

[:Gijs (he/him)]

(In reply to Alejandro Rodriguez [:Alopepeo] from comment #7)
> (In reply to :Gijs Kruitbosch from comment #6)
> > So I saw this once, tried to debug it, then promptly couldn't reproduce
> > anymore. Can't reproduce on a clean profile, either. Really unsure what
> > would cause this... :-\
> That's true, on the latest builds the problem seems to be gone, at least for
> now.....

Hang on. I was writing docs and suddenly had a small epiphany. You're not on Windows 8, are you? I wonder if this was the switch-to-metro button (which isn't available in private browsing mode), and was causing problems. This was recently disabled on all the non-win8 platforms (as it should have been from the beginning) and that'd explain why we all suddenly can't reproduce anymore.

Comment 11

[Alejandro Rodriguez [:Alopepeo]]

(In reply to :Gijs Kruitbosch from comment #10) 
> Hang on. I was writing docs and suddenly had a small epiphany. You're not on
> Windows 8, are you? I wonder if this was the switch-to-metro button (which
> isn't available in private browsing mode), and was causing problems. This
> was recently disabled on all the non-win8 platforms (as it should have been
> from the beginning) and that'd explain why we all suddenly can't reproduce
> anymore.

You're right! I'm running Windows 7 and the switch-to-metro button is no longer there since some time ago, so that's surely the cause of the problem. Nice catch!

Comment 12

[:Gijs (he/him)]

(In reply to Alejandro Rodriguez [:Alopepeo] from comment #11)
> (In reply to :Gijs Kruitbosch from comment #10) 
> > Hang on. I was writing docs and suddenly had a small epiphany. You're not on
> > Windows 8, are you? I wonder if this was the switch-to-metro button (which
> > isn't available in private browsing mode), and was causing problems. This
> > was recently disabled on all the non-win8 platforms (as it should have been
> > from the beginning) and that'd explain why we all suddenly can't reproduce
> > anymore.
> 
> You're right! I'm running Windows 7 and the switch-to-metro button is no
> longer there since some time ago, so that's surely the cause of the problem.
> Nice catch!

Well, only inasmuch as then this would still break on win8. But Blair just tried this on his metro machine and it doesn't, so that's not a great explanation... :-\

Comment 13

[Alejandro Rodriguez [:Alopepeo]]

Until this point, the problem was still there:
- http://hg.mozilla.org/mozilla-central/rev/4bf430d990e5
And in this one the issue seems to be solved:
- http://hg.mozilla.org/mozilla-central/rev/85694fd9b17c

Gijs, can you see something relevant between those revisions?

Comment 14

[:Gijs (he/him)]

(In reply to Alejandro Rodriguez [:Alopepeo] from comment #13)
> Until this point, the problem was still there:
> - http://hg.mozilla.org/mozilla-central/rev/4bf430d990e5
> And in this one the issue seems to be solved:
> - http://hg.mozilla.org/mozilla-central/rev/85694fd9b17c
> 
> Gijs, can you see something relevant between those revisions?

Thanks for bisecting this! There are actually quite a number of Australis-related changes inbetween those revisions. I'll look at this in detail tomorrow. The win8 metro button thing is also in there...

Comment 15

[:Gijs (he/him)]

Attachment: Australis' non-private-browsing-widget breaks customize mode,

Test + fix :-)

Comment 16

[:Gijs (he/him)]

So basically, I just don't think we should be passing these non-available widgets in if they are not, in fact, available in that window. Seemed like much the simplest solution.

Generally, I would otherwise assume widget build functions to not return null, and for XUL widgets, if they are in the palette then forWindow(...).node should be able to find them just fine.

Comment 17

[:Gijs (he/him)]

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

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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