Closed Bug 1102961 Opened 5 years ago Closed 5 years ago

Cannot navigate without closing the Options window

Categories

(Firefox :: Preferences, defect)

x86_64
Windows 7
defect
Not set
Points:
1

Tracking

()

VERIFIED FIXED
Firefox 37
Iteration:
37.3 - 12 Jan
Tracking Status
firefox33 --- unaffected
firefox34 + wontfix
firefox35 --- wontfix
firefox36 --- verified
firefox37 --- verified
firefox-esr31 --- unaffected

People

(Reporter: petruta.rasa, Assigned: abdelrahman, Mentored)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Reproduces in Firefox 34 beta 11 under Win 7 64-bit.

STR:
1. Select "Change Search Settings" from search toolbar arrow - Options window with Search tab selected is opened
2. Click on "Add more search providers..." link - https://addons.mozilla.org/en-US/firefox/search/?atype=4 page opens
3. Select a new search provider from the AMO list and add it to Firefox

ER: User can navigate to the AMO page (such as in Ubuntu and Mac OSX)

AR: The Options window remains in view and it must be closed before navigating to AMO page
Blocks: fx34
No longer blocks: fx34-searchui
Blocks: fx34-searchui
No longer blocks: fx34
Note: this problem goes away with in-content preferences.
This isn't a blocker for 34. I've marked 34 as wontfix and 35+ as affected. I don't think this needs to track.
Flags: firefox-backlog? → firefox-backlog+
Attachment #8540846 - Flags: review?(florian)
Assignee: nobody → a.ahmed1026
Status: NEW → ASSIGNED
Comment on attachment 8540846 [details] [diff] [review]
rev 1 - closing the options window

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

Thanks for the patch, and sorry for the delay in reviewing this!

::: browser/components/preferences/search.js
@@ +64,5 @@
>      Services.search.currentEngine =
>        document.getElementById("defaultEngine").selectedItem.engine;
>      this.displayOneClickEnginesList();
> +  },
> +  

Nit: please avoid adding trailing whitespace.

@@ +67,5 @@
> +  },
> +  
> +  loadAddEngines: function () {
> +    window.opener.BrowserSearch.loadAddEngines();
> +    window.close();

This will not save any change the user made. To save the changes and close the window, you can use instead:
  window.document.documentElement.acceptDialog();

::: browser/components/preferences/search.xul
@@ +50,5 @@
>        <label>The search bar lets you search alternate engines directly. Choose which ones to display.</label>
>  
>        <richlistbox id="oneClickProvidersList"/>
>        <hbox pack="end" style="margin-bottom: 1em">
>          <label id="addEngines" class="text-link" value="Add more search providers…"

I think this patch was based on an old revision of the code, strings are no longer hardcoded.
Attachment #8540846 - Flags: review?(florian)
Attachment #8540846 - Flags: review-
Attachment #8540846 - Flags: feedback+
Attachment #8540846 - Attachment is obsolete: true
Attachment #8544543 - Flags: review?(florian)
Comment on attachment 8544543 [details] [diff] [review]
rev 2 - closing the options window

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

Thanks for updating the patch! :-)

::: browser/components/preferences/search.js
@@ +183,5 @@
> +  },
> +
> +  loadAddEngines: function () {
> +    window.opener.BrowserSearch.loadAddEngines();
> +    window.document.documentElement.acceptDialog();

Note: this won't close the dialog on Mac (due to http://hg.mozilla.org/mozilla-central/annotate/2a193b7f395c/toolkit/content/widgets/preferences.xml#l637) but I don't think that's a problem. The bug we are trying to fix here is really that on Windows it's currently not possible to browse the AMO page after clicking the link; closing the preferences dialog is not a goal in itself.
Attachment #8544543 - Flags: review?(florian) → review+
Attachment #8544543 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/4f0b4498fae5
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Iteration: --- → 37.3 - 12 Jan
Flags: qe-verify?
Points: --- → 1
Flags: qe-verify? → qe-verify+
Comment on attachment 8544543 [details] [diff] [review]
rev 2 - closing the options window

Approval Request Comment
[Feature/regressing bug #]: bug 1088660
[User impact if declined]: Clicking the "add more engine" link in the Search prefpane looks broken on Windows because the modal pref dialog stays above the new tab that's opened.
[Describe test coverage new/current, TBPL]: currently on m-c, will be verified by QA.
[Risks and why]: low, self contained change.
[String/UUID change made/needed]: none.
Attachment #8544543 - Flags: approval-mozilla-aurora+ → approval-mozilla-beta?
QA Contact: camelia.badau
Verified fixed on Windows 7 using latest Aurora 37.0a2 (buildID: 20150113004007).
Status: RESOLVED → VERIFIED
Attachment #8544543 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified fixed on Windows 7 64bit and 32bit using Firefox 36 Beta 2 (buildID: 20150120155007).
You need to log in before you can comment on or make changes to this bug.