Closed
Bug 885351
Opened 11 years ago
Closed 11 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)
Tracking
()
VERIFIED
FIXED
Firefox 25
People
(Reporter: ioana_damy, Assigned: Gavin)
References
Details
Attachments
(1 file)
(deleted),
patch
|
MattN
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Thinking about this bug, should this not be the expected case? Should we not fall back to the default in case of failure?
Reporter | ||
Comment 2•11 years ago
|
||
(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.
Assignee | ||
Comment 4•11 years ago
|
||
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.
status-firefox23:
--- → affected
tracking-firefox23:
--- → ?
Assignee | ||
Comment 6•11 years ago
|
||
I'm going back and forth on whether this is worth fixing "soon", it may be complicated.
Assignee | ||
Updated•11 years ago
|
Assignee | ||
Comment 7•11 years ago
|
||
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 8•11 years ago
|
||
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+
Assignee | ||
Comment 9•11 years ago
|
||
Target Milestone: --- → Firefox 25
Assignee | ||
Comment 10•11 years ago
|
||
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?
Comment 11•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 12•11 years ago
|
||
Ioana, can you please verify this is fixed in today's Nightly?
Keywords: verifyme
QA Contact: ioana.budnar
Updated•11 years ago
|
Attachment #769806 -
Flags: approval-mozilla-beta?
Attachment #769806 -
Flags: approval-mozilla-beta+
Attachment #769806 -
Flags: approval-mozilla-aurora?
Attachment #769806 -
Flags: approval-mozilla-aurora+
Comment 13•11 years ago
|
||
Reporter | ||
Comment 14•11 years ago
|
||
Verified as fixed on Firefox 23.0b3 (07/03), 24.0a2 (07/04) and 25.0a1 (07/04).
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•