774660 - openPreferences should consistently not return a window object

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[(no longer active)]

See <http://mxr.mozilla.org/mozilla-central/source/browser/base/content/utilityOverlay.js#477>.  This will be a problem when we use the return value in the test case for bug 722978.

Comment 1

[Dão Gottwald [:dao]]

The current return value appears to be unused and returning a window is non-trivial in the in-content case, so we should instead remove it. The test in bug 722978 should use observers or something.

Comment 2

[(no longer active)]

(In reply to comment #1)
> The current return value appears to be unused and returning a window is
> non-trivial in the in-content case, so we should instead remove it. The test in
> bug 722978 should use observers or something.

How would using observers get us to the right window object?

Comment 3

[Dão Gottwald [:dao]]

(In reply to Ehsan Akhgari [:ehsan] from comment #2)
> (In reply to comment #1)
> > The current return value appears to be unused and returning a window is
> > non-trivial in the in-content case, so we should instead remove it. The test in
> > bug 722978 should use observers or something.
> 
> How would using observers get us to the right window object?

Via the subject argument, or the test would just look for the window itself using window watcher and tabbrowser APIs.

Comment 4

[Saurabh Anand [:sawrubh]]

@Dao, @Ehsan, will this bug block bug 722978 ?

BTW, I think I can work on this :)

Comment 5

[Dão Gottwald [:dao]]

(In reply to Saurabh Anand [:sawrubh] from comment #4)
> @Dao, @Ehsan, will this bug block bug 722978 ?

No, I morphed it. This bug is now about making openPreferences never return a window.

Comment 6

[Saurabh Anand [:sawrubh]]

Attachment: Patch v1

Comment 7

[(no longer active)]

(In reply to comment #4)
> @Dao, @Ehsan, will this bug block bug 722978 ?

Hmm, I think so, yes, since now you cannot use the openPreferences() function in a straightforward manner there.

> BTW, I think I can work on this :)

Great!

Comment 8

[Saurabh Anand [:sawrubh]]

Attachment: Patch v2

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 643378 [details] [diff] [review]
Patch v2

>+    openDialog("chrome://browser/content/preferences/preferences.xul",
>                       "Preferences", features, paneID, extraArgs);

Looks good except for the indentation in the second line here.

Comment 10

[Saurabh Anand [:sawrubh]]

Attachment: Patch v3

Fixed indentation.

Comment 11

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/61b822ad93a1

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/61b822ad93a1