openPreferences should consistently not return a window object

RESOLVED FIXED in Firefox 17

Status

()

Firefox
Preferences
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ehsan, Assigned: sawrubh)

Tracking

unspecified
Firefox 17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=dao][lang=js])

Attachments

(1 attachment, 2 obsolete attachments)

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.
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.
OS: Mac OS X → All
Hardware: x86 → All
Summary: openPreferences no longer returns the window object for the preferences window when in-content prefs are active → openPreferences should consistently not return a window object
(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?
(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.

Updated

5 years ago
Whiteboard: [good first bug][mentor=dao][lang=js]
(Assignee)

Comment 4

5 years ago
@Dao, @Ehsan, will this bug block bug 722978 ?

BTW, I think I can work on this :)
(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.
(Assignee)

Comment 6

5 years ago
Created attachment 643365 [details] [diff] [review]
Patch v1
Assignee: nobody → saurabhanandiit
Status: NEW → ASSIGNED
Attachment #643365 - Flags: feedback?(dao)
(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!
(Assignee)

Comment 8

5 years ago
Created attachment 643378 [details] [diff] [review]
Patch v2
Attachment #643365 - Attachment is obsolete: true
Attachment #643365 - Flags: feedback?(dao)
Attachment #643378 - Flags: feedback?(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.
Attachment #643378 - Flags: feedback?(dao) → feedback+
(Assignee)

Comment 10

5 years ago
Created attachment 643391 [details] [diff] [review]
Patch v3

Fixed indentation.
Attachment #643378 - Attachment is obsolete: true
Attachment #643391 - Flags: review?(dao)

Updated

5 years ago
Attachment #643391 - Flags: review?(dao) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/61b822ad93a1
Target Milestone: --- → Firefox 17
https://hg.mozilla.org/mozilla-central/rev/61b822ad93a1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.