bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Search engines: keywords and order gone after upgrading from 44.0.2 to 45.0 with language pack active

RESOLVED FIXED in Firefox 45

Status

()

Firefox
Search
P1
major
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: aryx, Assigned: florian)

Tracking

({dataloss, regression})

45 Branch
Firefox 48
dataloss, regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox45blocking verified, firefox46 fixed, firefox47 fixed, firefox48 fixed, firefox-esr45 verified, relnote-firefox 45+)

Details

(Whiteboard: [fxsearch] investigation)

Attachments

(2 attachments, 2 obsolete attachments)

After upgrading from Firefox 44.0.2 64 bit to 45.0 64 bit on a Windows 8.1 machine, my search engines have all lost their keywords and their order (it's alphabetic now). I wasn't able to reproduce with a new profile created in 44.0.2 and launching that in 45.0 but will test with a copy of my backup tomorrow.

Further information:
- The profile is very old. But this shouldn't matter since the search engines got migrated from files to a database like structure.
- Some of the search engines got added with the extension 'Add To Search Bar': https://addons.mozilla.org/firefox/addon/add-to-search-bar/
- amazon.de is twice in my list of search engines (might have been done by myself): Once as 'amazon.de', once as 'Amazon.de'
Steps to reproduce:
1. Create a new profile in Firefox 44.0.2.
2. Install the German language pack from https://ftp.mozilla.org/pub/firefox/releases/44.0.2/win32/xpi/de.xpi
3. Open about:config.
4. Set general.useragent.locale to 'de'
5. Close and relaunch Firefox.
6. Open search settings.
7. Set keyword for Bing search to 'bg'.
8. Close Firefox.
9. Launch profile in Firefox 45.0 and accept to upgrade for the language pack.
10. Open search settings.
Actual result:
Search keyword for Bing is gone (search engines have still their order). If I reorder the search engines in step 7, they will be order alphabetically after step 10.
There is no error in the Browser Console.
If the language pack doesn't get updated in step 9, keyword and order are untouched after step 10.
Expected result:
Search engines unaltered.

See also bug 1236347 which is also about issues with language packs starting in Firefox 45.
Severity: normal → major
status-firefox45: --- → affected
status-firefox46: --- → ?
status-firefox47: --- → ?
status-firefox48: affected → ?
tracking-firefox45: --- → ?
Keywords: dataloss
Version: Trunk → 45 Branch
Summary: Search engines: keywords and order gone after upgrading from 44.0.2 to 45.0 → Search engines: keywords and order gone after upgrading from 44.0.2 to 45.0 with language pack active
Florian, any clue? Merci
Flags: needinfo?(florian)
(Assignee)

Comment 3

2 years ago
This is likely a side effect of bug 1203167 (large search service refactoring that landed for Firefox 45).
Blocks: 1203167

Comment 4

2 years ago
will demote if appropriate after understand what is happening and how large the impact - scope.
Priority: -- → P1
Whiteboard: [fxsearch] investigation

Comment 5

2 years ago
Florian, just passing on some information from JorgeV as others noticed this bug being flagged as well
 
<jorgev> this should give you a rough idea https://addons.mozilla.org/en-US/firefox/search/?appver=45.0&atype=5
<jorgev> potentially millions
<kev> yeah, don't know how many people change order of searches and/or use keywords
 — eviljeff suspects few of the former and very few for the latter
 <kev> same but should be evaluated, all the same
<jorgev> also, it looks like the real current usage of these language packs is much higher than the average
:aryx pointed me to this bug. I'm using the en-GB locale build with no language packs installed.
(Assignee)

Comment 7

2 years ago
(In reply to Kohei Yoshino [:kohei] from comment #6)
> :aryx pointed me to this bug. I'm using the en-GB locale build with no
> language packs installed.

Do you have specific steps to reproduce that don't involve a language pack?
Flags: needinfo?(kohei.yoshino)
I have just upgraded from 44.0.2 to 45. And I don't have search engine related extensions neither. No extra steps.

My default search provider is DuckDuckGo that remains as-is. The order of other search providers, Google was first in my case, was lost, and some of my disabled search providers, Bing and Chanbers (UK), were enabled once upgraded.
Flags: needinfo?(kohei.yoshino)
(Assignee)

Comment 9

2 years ago
(In reply to Kohei Yoshino [:kohei] from comment #8)
> I have just upgraded from 44.0.2 to 45. And I don't have search engine
> related extensions neither. No extra steps.

I can't reproduce, so there must be a detail or a step missing.

> some of
> my disabled search providers, Bing and Chanbers (UK), were enabled once
> upgraded.

What do you mean with "enabled"?
(Assignee)

Comment 10

2 years ago
Created attachment 8728922 [details] [diff] [review]
Fix v1

One-line fix. I'll try to write a test.
(Assignee)

Updated

2 years ago
Assignee: nobody → florian
Status: NEW → ASSIGNED
(Assignee)

Comment 11

2 years ago
Created attachment 8728932 [details] [diff] [review]
Fix for synchronous code path

Someone very helpfully emailed me the relevant parts of a profile that could reproduce the issue without lang pack involved. This profile contains lots of user-installed engines (42), and for some reason the synchronous initialization code path gets triggered. Turns out that code path isn't currently covered by tests, and had trivial mistakes.
(In reply to Florian Quèze [:florian] [:flo] from comment #9)
> > some of
> > my disabled search providers, Bing and Chanbers (UK), were enabled once
> > upgraded.
> 
> What do you mean with "enabled"?

Those providers have appeared in the search bar's drop down list.

I have "user-installed engines" but only 4, including Bugzilla@Mozilla (this site), FxSiteCompat.com (my site), MDN and GitHub.
(Assignee)

Comment 13

2 years ago
Do you have an add-on enabled that forces a synchronous initialization of the search service? (This causes a warning in the browser console)

If you would like me to try to reproduce with your profile, you can email me these files from the profile:
- search.json
- search-metadata.json
- the searchplugins/ folder
(Assignee)

Comment 14

2 years ago
Created attachment 8728957 [details] [diff] [review]
Fix synchronous code path v2 (with test)

Note: Surprisingly, the patch was developped against mozilla-release, but also applies cleanly to the current fx-team.
Attachment #8728957 - Flags: review?(adw)
(Assignee)

Updated

2 years ago
Attachment #8728932 - Attachment is obsolete: true
(Assignee)

Comment 15

2 years ago
Created attachment 8729028 [details] [diff] [review]
Fix for engines from language packs, v2 (with test)
Attachment #8729028 - Flags: review?(adw)
(Assignee)

Updated

2 years ago
Attachment #8728922 - Attachment is obsolete: true

Comment 17

2 years ago
OK, the loss of order and keywords happened before -- annoying but I have already made a txt list that helps me to restore it manually.

However with this latest update (v45.0) I cannot even edit the keywords! There's no option for that. Extremely annoying as I do a lot of research and use them hundreds of times every day.

How is this related to a language pack anyway? I am using English Mozilla.. I think I only have additional Russian spell-checking dictionary installed.

Florian, can I use the fixes you provided and how?
(Assignee)

Comment 18

2 years ago
(In reply to Egon Upitis from comment #17)

> However with this latest update (v45.0) I cannot even edit the keywords!
> There's no option for that.

See bug 1138112.

> How is this related to a language pack anyway? I am using English Mozilla..
> I think I only have additional Russian spell-checking dictionary installed.

If you think you are encountering a different case I may not have identified yet, and would like me to try to reproduce with the search settings of the profile, you can email me the relevant files (see comment 13).

> Florian, can I use the fixes you provided and how?

Not until they are released.
Flags: needinfo?(florian)

Comment 19

2 years ago
Thanks, Florian -- I thought I tried everything, glad the keywords still work.

I think my case is the same as https://bugzilla.mozilla.org/show_bug.cgi?id=1254694#c11 -- lots of search engines installed. In fact I am now removing some that I don't ever use.

Updated

2 years ago
Attachment #8728957 - Flags: review?(adw) → review+

Updated

2 years ago
Attachment #8729028 - Flags: review?(adw) → review+
(Assignee)

Comment 20

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/eed38188ee0ef70c90db4767dd614508a1a9042c
Bug 1254694 - Fix synchronous code path for search engines metadata migration, r=adw.

https://hg.mozilla.org/integration/fx-team/rev/0837015740ff57ed9c40a6733987f437985b9d9b
Bug 1254694 - Fix search engines metadata migration for engines from language packs, r=adw.

Comment 21

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eed38188ee0e
https://hg.mozilla.org/mozilla-central/rev/0837015740ff
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: ? → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
(Assignee)

Comment 22

2 years ago
Comment on attachment 8728957 [details] [diff] [review]
Fix synchronous code path v2 (with test)

Approval Request Comment
[Feature/regressing bug #]: bug 1203167
[User impact if declined]: users with lots of user-installed search plugins are likely to lose search engine metadata (aliases, order, removed default engine).
[Describe test coverage new/current, TreeHerder]: the patch contains an xpcshell test.
[Risks and why]: very low, the patch fixes trivial mistakes in a code path that isn't used in the general case, and that's wrapped in a try/catch block. The code touched by the patch is completely broken without the patch.
[String/UUID change made/needed]: none
Attachment #8728957 - Flags: approval-mozilla-release?
Attachment #8728957 - Flags: approval-mozilla-beta?
Attachment #8728957 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 23

2 years ago
Comment on attachment 8729028 [details] [diff] [review]
Fix for engines from language packs, v2 (with test)

Approval Request Comment
[Feature/regressing bug #]: bug 1203167
[User impact if declined]: users using a language pack are likely to lose search engine metadata (aliases, order, removed default engine).
[Describe test coverage new/current, TreeHerder]: the patch contains an xpcshell test.
[Risks and why]: low; one line fix, modifying the engine id, which is only ever used during metadata migration.
[String/UUID change made/needed]: none
Attachment #8729028 - Flags: approval-mozilla-release?
Attachment #8729028 - Flags: approval-mozilla-beta?
Attachment #8729028 - Flags: approval-mozilla-aurora?
Comment on attachment 8728957 [details] [diff] [review]
Fix synchronous code path v2 (with test)

Fix a migration issue, taking it on all branches.
Attachment #8728957 - Flags: approval-mozilla-release?
Attachment #8728957 - Flags: approval-mozilla-release+
Attachment #8728957 - Flags: approval-mozilla-esr45+
Attachment #8728957 - Flags: approval-mozilla-beta?
Attachment #8728957 - Flags: approval-mozilla-beta+
Attachment #8728957 - Flags: approval-mozilla-aurora?
Attachment #8728957 - Flags: approval-mozilla-aurora+
Attachment #8729028 - Flags: approval-mozilla-release?
Attachment #8729028 - Flags: approval-mozilla-release+
Attachment #8729028 - Flags: approval-mozilla-esr45+
Attachment #8729028 - Flags: approval-mozilla-beta?
Attachment #8729028 - Flags: approval-mozilla-beta+
Attachment #8729028 - Flags: approval-mozilla-aurora?
Attachment #8729028 - Flags: approval-mozilla-aurora+
Flags: in-testsuite+
We should perform further Exploratory Testing around this scenario.
Flags: qe-verify+

Updated

2 years ago
Duplicate of this bug: 1256737
Added to the release notes with "Fix a regression causing search engine metadata to be lost in some context (1254694)" as wording
tracking-firefox45: ? → blocking
relnote-firefox: --- → 45+
(Assignee)

Comment 32

2 years ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #31)
> Added to the release notes with "Fix a regression causing search engine
> metadata to be lost in some context (1254694)" as wording

"metadata" seems a bit jargon-ish to me, maybe "settings" instead would be more understandable?
Right, updated, merci!
I. 
Initial Scenario:  Using steps from comment 1 and referring only to the update scenario which breaks search settings.
44.02 (update to) 45.0
44.02 (update to) 45.0.1
45ESR (update to) 45.0.1 ESR

Windows10 -> reproducible
Mac -> not reproducible
Ubuntu -> reproducible

The update/migration bug can be verified fixed on WINDOWS and UBUNTU for 45.0.1, 45.0.1 ESR with the note that (comment 1) wasn't reproducible on MAC. 

II. 
What I noticed for all OS'es was that installing the language pack and setting the " general.useragent.locale to 'de' ", then just restarting FF was causing the search engines settings to break. (behavior reproducible for 44.02, 45, 45.01, 45.01ESR)


I also went back a bit and checked if I. and II. scenarios are reproducible for older releases: 42.0 and 43.0.1 with the following result: I. No  II. Yes.
Please advise if scenario II. should be split up into a separate issue.
Flags: needinfo?(florian)
(Assignee)

Comment 35

2 years ago
(In reply to Adrian Florinescu [:AdrianSV] from comment #34)

> II. 
> What I noticed for all OS'es was that installing the language pack and
> setting the " general.useragent.locale to 'de' ", then just restarting FF
> was causing the search engines settings to break. (behavior reproducible for
> 44.02, 45, 45.01, 45.01ESR)
> 
> 
> I also went back a bit and checked if I. and II. scenarios are reproducible
> for older releases: 42.0 and 43.0.1 with the following result: I. No  II.
> Yes.
> Please advise if scenario II. should be split up into a separate issue.

Unless I'm misunderstanding something, II. may not be a real issue. When you switch to a different language pack, you switch to a different set of default search engines, so settings can't really be kept, or at least not all of them. We may want to try to preserve the settings automatically for engines that have the same name for both languages (eg. 'Google'), but that's not something we should attempt to do in a dot release. Us losing some settings when the user actively switches back and forth between languages is very different from (and less problematic than) losing settings during an update.
Flags: needinfo?(florian)
I agree that the scope of this bug was to track and fix the version update issue that was ruining the search settings. In regards to it, the fix can be marked as verified;
I can also confirm that the bug does not regress on build 2 for 45.0.1 ESR:

45.0.1 Build-ID  20160316151906 User-Agent  Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
45.0.1Build-ID 20160316151906 Mozilla/5.0 (X11; Linux i686; rv:45.0) Gecko/20100101 Firefox/45.0
45.0.1 Build-ID 20160316151906 User-Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Firefox/45.0

Adding the above to comment 34 /point I., we can mark this issues as verified on: Ubuntu14.04/Mac10.11/Windows10 for Release 45.0.1 and 45.0.1ESR (build1+build2)
status-firefox45: fixed → verified
status-firefox-esr45: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.