Last Comment Bug 774660 - openPreferences should consistently not return a window object
: openPreferences should consistently not return a window object
Status: RESOLVED FIXED
[good first bug][mentor=dao][lang=js]
:
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Firefox 17
Assigned To: Saurabh Anand [:sawrubh]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-17 07:45 PDT by :Ehsan Akhgari (busy, don't ask for review please)
Modified: 2012-07-19 07:35 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (1.33 KB, patch)
2012-07-18 07:17 PDT, Saurabh Anand [:sawrubh]
no flags Details | Diff | Review
Patch v2 (1.36 KB, patch)
2012-07-18 08:07 PDT, Saurabh Anand [:sawrubh]
dao+bmo: feedback+
Details | Diff | Review
Patch v3 (1.42 KB, patch)
2012-07-18 08:29 PDT, Saurabh Anand [:sawrubh]
dao+bmo: review+
Details | Diff | Review

Description :Ehsan Akhgari (busy, don't ask for review please) 2012-07-17 07:45:36 PDT
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] 2012-07-17 08:07:42 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-17 19:46:27 PDT
(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] 2012-07-18 02:36:41 PDT
(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] 2012-07-18 04:15:46 PDT
@Dao, @Ehsan, will this bug block bug 722978 ?

BTW, I think I can work on this :)
Comment 5 Dão Gottwald [:dao] 2012-07-18 06:12:53 PDT
(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] 2012-07-18 07:17:10 PDT
Created attachment 643365 [details] [diff] [review]
Patch v1
Comment 7 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-18 07:32:46 PDT
(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] 2012-07-18 08:07:01 PDT
Created attachment 643378 [details] [diff] [review]
Patch v2
Comment 9 Dão Gottwald [:dao] 2012-07-18 08:13:44 PDT
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] 2012-07-18 08:29:57 PDT
Created attachment 643391 [details] [diff] [review]
Patch v3

Fixed indentation.
Comment 11 :Ehsan Akhgari (busy, don't ask for review please) 2012-07-18 09:42:10 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/61b822ad93a1
Comment 12 Ed Morley [:emorley] 2012-07-19 07:35:18 PDT
https://hg.mozilla.org/mozilla-central/rev/61b822ad93a1

Note You need to log in before you can comment on or make changes to this bug.