1019828 - in-content preferences won't open if no window open in OSX

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Hernán Rodriguez Colmeiro (:peregrino)]

If you try to open the preferences panel either via the hotkey cmd+, or from Nightly->Preferences, it won't open if there's no window already open.

Comment 1

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

This isn't a regression, but the severity of it doesn't warrant blocking.

Comment 2

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

Hernan said that he will look in to this.

Comment 4

[:Gijs (he/him)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> This isn't a regression, but the severity of it doesn't warrant blocking.

Er, it technically is - on e.g. 33, if I close windows and hit cmd-',' the preferences window opens.

Hernán, are you still looking at this? :-)

Comment 5

[Hernán Rodriguez Colmeiro (:peregrino)]

I do want to keep looking at this, but I'm in the middle of the initial release of the main product of my day job next week, so kind of busy ATM :)

Comment 6

[dawa]

Have created account because this is the only bug I can see that relates to my problem. Every iteration release above 31.0 has a failed Preferences pane, and it is regardless of an open window or not. 
I submitted feedback and have screenshots going back to september for two instances- v32 and 33x I believe. How is this NOT a signifcant failure of the app? The prefs must be used frequently to trash cache etc.- dave

Comment 7

[:Gijs (he/him)]

(In reply to dawa from comment #6)
> Have created account because this is the only bug I can see that relates to
> my problem. Every iteration release above 31.0 has a failed Preferences
> pane, and it is regardless of an open window or not. 

Can you please file a new bug, and include your screenshots and a more detailed description of what you mean by "a failed Preferences pane"? I've never heard of this before, and yes, it does sound very serious - but not everyone is seeing this issue, and without more details about your situation it's impossible to fix it. Please file a new bug here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Firefox&component=Preferences and add me in the "CC" field. Thank you!

Comment 8

[:Gijs (he/him)]

Stealing!

Comment 9

[:Gijs (he/him)]

Attachment: openPreferences should also work if no windows are open,

This works in my testing. Code shamelessly cribbed from openPreferences in BrowserContentHandler.jsm.

Comment 10

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #9)
> Created attachment 8572649 [details] [diff] [review]
> openPreferences should also work if no windows are open,
> 
> This works in my testing. Code shamelessly cribbed from openPreferences in
> BrowserContentHandler.jsm.

Err, by which I mean, nsBrowserContentHandler.js

Comment 11

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

Comment on attachment 8572649 [details] [diff] [review]
openPreferences should also work if no windows are open,

Review of attachment 8572649 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/utilityOverlay.js
@@ +514,5 @@
>      return (aName || "").replace(/^pane./, function(toReplace) { return toReplace[4].toLowerCase(); });
>    }
>  
>    if (getBoolPref("browser.preferences.inContent")) {
> +    let friendlyCategoryName = internalPrefCategoryNameToFriendlyName(paneID);

It looks like this line can be moved back to after `let win = Services.wm.getMostRecentWindow` to keep the diff smaller.

@@ +521,5 @@
>                           (friendlyCategoryName ? "#" + friendlyCategoryName : "");
> +    let newLoad = true;
> +    let browser = null;
> +    if (!win) {
> +

nit, remove this blank line.

@@ +527,5 @@
> +                         .createInstance(Components.interfaces.nsISupportsArray);
> +
> +      var wuri = Components.classes["@mozilla.org/supports-string;1"]
> +                           .createInstance(Components.interfaces.nsISupportsString);
> +      wuri.data = preferencesURL;

Wow, this is pretty ugly (not your fault). Can we find some better variable names though? Also, it looks like these can be switched to `let` since they're not referenced outside of this block.

Comment 12

[:Gijs (he/him)]

w/ nits addressed:

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

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/65d1053d7959

Comment 14

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Mac OSX 10.9.5 using latest Nightly 39.0a1 (buildID: 20150310030235).

Comment 15

[:Gijs (he/him)]

Comment on attachment 8572649 [details] [diff] [review]
openPreferences should also work if no windows are open,

Approval Request Comment
[Feature/regressing bug #]: in-content prefs
[User impact if declined]: can't open prefs on OS X when no windows are open, which breaks platform conventions
[Describe test coverage new/current, TreeHerder]: nope, can't close main window in most/(all?) test suites we have
[Risks and why]: pretty low, fairly specific fix which reuses a bunch of code we already had anyway
[String/UUID change made/needed]: nope

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b1162f882005

Comment 17

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Mac OSX 10.9.5 using latest Aurora 38.0a2 (buildID: 20150316004007).