Closed Bug 885351 Opened 7 years ago Closed 7 years ago

Google is always used in about:home and the URL bar when setting the default search engine to an invalid one

Categories

(Firefox :: Search, defect)

23 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 25
Tracking Status
firefox23 + verified
firefox24 + verified
firefox25 --- verified

People

(Reporter: ioana_damy, Assigned: Gavin)

References

Details

Attachments

(1 file)

Reproducible on:
Mozilla/5.0 (X11; Linux i686; rv:23.0) Gecko/20130620 Firefox/23.0
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20130620 Firefox/24.0

STR:
1. Launch Firefox.
2. Change the search engine in the search bar to anything except Google.
3. Restart Firefox (this step is only necessary on Fx23, not on 24).
4. Go to about:config and change browser.search.defaultenginename to a random string.
5. Restart Firefox.

Expected Results:
The default search engine stays the same in the search bar, URL bar and about:home - the engine selected at step 2.

Actual Results:
The search bar keeps the last selected engine (step 2), while about:home and the URL bar use Google.
Blocks: 738818
Thinking about this bug, should this not be the expected case? Should we not fall back to the default in case of failure?
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #1)
> Thinking about this bug, should this not be the expected case? Should we not
> fall back to the default in case of failure?

It would. When I selected an engine at step 2, that engine was marked as default per bug 738818. The more concerning part for me is having different engines in different places (see  comment 0 Actual Results) though.
Gavin, what do you think?
Flags: needinfo?(gavin.sharp)
Yes, definitely a valid bug. If an addon messes with your search prefs you shouldn't be able to get into this state.
Flags: needinfo?(gavin.sharp)
Given this relates back to bug 738818 which has been rel-noted for Firefox 23 I'm nominating this for tracking.
I'm going back and forth on whether this is worth fixing "soon", it may be complicated.
Assignee: nobody → gavin.sharp
Blocks: 880882
Attached patch patchSplinter Review
This should fix both this bug and bug 880882, by ensuring on every startup that defaultEngine == currentEngine. It does this by observing search service initialization, and syncing those properties immediately (the patch in bug 860560 ensures that they then remain in sync).

This has the potential to conflict with search add-ons that try to set defaultenginename on every startup, but that is somewhat incompatible with the design implemented in bug 738818 anyways.
Attachment #769806 - Flags: review?(mnoorenberghe+bmo)
Comment on attachment 769806 [details] [diff] [review]
patch

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

::: browser/components/nsBrowserGlue.js
@@ +325,5 @@
>          // its currentEngine. The search service will notify us any time either
>          // of them are changed (either by directly setting the relevant prefs,
>          // i.e. if add-ons try to change this directly, or if the
>          // nsIBrowserSearchService setters are called).
> +        // No need to initialize the search service, since it's garanteed to be

Nit: s/garanteed/guaranteed/

@@ +422,5 @@
>      os.removeObserver(this, "keyword-search");
>  #endif
>      os.removeObserver(this, "browser-search-engine-modified");
> +    try {
> +      os.removeObserver(this, "browser-search-service");

We could try checking Services.search.isInitialized here instead of the try…catch but the try…catch is more defensive which is good.
Attachment #769806 - Flags: review?(mnoorenberghe+bmo) → review+
Comment on attachment 769806 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 738818
User impact if declined: existing users and users of add-ons may encounter discrepancies between search bar and other search forms, which goes against the design in bug 738818
Testing completed (on m-c, etc.): manually
Risk to taking this patch (and alternatives if risky): low-risk
String or IDL/UUID changes made by this patch: none
Attachment #769806 - Flags: approval-mozilla-beta?
Attachment #769806 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/2f349618cab1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Ioana, can you please verify this is fixed in today's Nightly?
Keywords: verifyme
QA Contact: ioana.budnar
Attachment #769806 - Flags: approval-mozilla-beta?
Attachment #769806 - Flags: approval-mozilla-beta+
Attachment #769806 - Flags: approval-mozilla-aurora?
Attachment #769806 - Flags: approval-mozilla-aurora+
Verified as fixed on Firefox 23.0b3 (07/03), 24.0a2 (07/04) and 25.0a1 (07/04).
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.