Update overwrites search engine preferences for users of en-US langpack

VERIFIED FIXED in Firefox 58

Status

()

VERIFIED FIXED
a year ago
10 months ago

People

(Reporter: guillaume, Assigned: florian)

Tracking

57 Branch
Firefox 59
Points:
---

Firefox Tracking Flags

(firefox58 verified, firefox59 verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171128222554

Steps to reproduce:

Updated Firefox from 57 to 57.0.1 when asked to by the browser.


Actual results:

Google became the default search engine, despite my previous settings that had entirely removed it from the list.


Expected results:

The default search engine I had chosen in version 57 should have remained the default one in version 57.0.1.
Moreover, Google that had been removed in version 57 should not have reappeared in version 57.0.1 without me explicitly requesting that. It should not be set as default either.

Updated

a year ago
Component: Untriaged → Search

Comment 1

11 months ago
This sounds like bug 1419941.  See especially the first and second comments in that bug.  Was your previous default engine installed by an add-on or third party?
Flags: needinfo?(guillaume)
(Assignee)

Comment 2

11 months ago
(In reply to Guillaume from comment #0)

> The default search engine I had chosen in version 57 should have remained
> the default one in version 57.0.1.

Are you sure you selected that engine as the default using version 57 and not an older one?
(Reporter)

Comment 3

11 months ago
(In reply to Drew Willcoxon :adw from comment #1)
> This sounds like bug 1419941.  See especially the first and second comments
> in that bug.  Was your previous default engine installed by an add-on or
> third party?

I had manually set up my previous search engine (DuckDuckGo), even before Firefox 57. In 57, I chose to remove all others from my list because I realized I only use DuckDuckGo now. At some point I installed the DuckDuckGo extension (in Firefox 57), but it was already offered as a WebExtension.

So, I believe my case should have fallen into this category mentioned in bug 1419941:

> - users currently having as default an engine known to come from an https origin will be offered the about:searchreset UI the next time they use one of our search access points. The reason why we need to prompt here is that it's likely that a large portion of these users have intentionally installed the engine themselves through open search discovery, and we don't want to silently undo user choice.
Flags: needinfo?(guillaume)
(Reporter)

Comment 4

11 months ago
(In reply to Florian Quèze [:florian] from comment #2)
> (In reply to Guillaume from comment #0)
> 
> > The default search engine I had chosen in version 57 should have remained
> > the default one in version 57.0.1.
> 
> Are you sure you selected that engine as the default using version 57 and
> not an older one?

I had been using DuckDuckGo as default since before Firefox 57, for sure. But I can't remember if this was the case before version 45.
Also, I never installed it, I simply selected it from the list offered by Firefox, which is why I find abnormal to find it reset to Google after a minor version update.
(Assignee)

Comment 5

11 months ago
If you would like me to check what happened for you, you can set your default engine back to what it was before the reset, then go to about:telemetry, and in the "Environment Data" section, look for the defaultSearchEngineData.loadPath line. Show me what's listed there. You can also go in about:config and check the value of the browser.search.reset.status preference.
(Reporter)

Comment 6

11 months ago
(In reply to Florian Quèze [:florian] from comment #5)
> If you would like me to check what happened for you, you can set your
> default engine back to what it was before the reset, then go to
> about:telemetry, and in the "Environment Data" section, look for the
> defaultSearchEngineData.loadPath line. Show me what's listed there. You can
> also go in about:config and check the value of the
> browser.search.reset.status preference.

Here is what I have under about:telemetry

defaultSearchEngine 	             other-DuckDuckGo
defaultSearchEngineData.name 	     DuckDuckGo
defaultSearchEngineData.loadPath     jar:[profile]/extensions/langpack-en-US@firefox.mozilla.org.xpi!browser/ddg.xml
defaultSearchEngineData.origin 	     verified

Looks normal to me.

But in about:config, this is what I have:

browser.search.reset.status    modified    string    silent

I suppose I need to change is to avoid future resets of the default search engine.
(Assignee)

Comment 7

11 months ago
(In reply to Guillaume from comment #6)

> Here is what I have under about:telemetry

Thanks!

> defaultSearchEngineData.loadPath    
> jar:[profile]/extensions/langpack-en-US@firefox.mozilla.org.xpi!browser/ddg.
> xml

Pretty unusual. This means you are using an en-US language pack to "translate" your Firefox into en-US.

Is this something you have installed? Is your build not an en-US one already?

If you are using an en-US language pack on an already en-US build, it's a case our code doesn't cover.

> browser.search.reset.status    modified    string    silent
> 
> I suppose I need to change is to avoid future resets of the default search
> engine.

You don't need to change this, this pref is written mostly so that we can check what happened after the fact. We are currently not planning another reset in the near future.
Blocks: 1419941
Summary: Update overwrites search engine preferences → Update overwrites search engine preferences for users of en-US langpack
(Assignee)

Comment 8

11 months ago
To fix this, I think at https://searchfox.org/mozilla-central/rev/f5f1c3f294f89cfd242c3af9eb2c40d19d5e04e7/browser/components/nsBrowserGlue.js#2201 we could also return early if:
  !Services.search.originalDefaultEngine.wrappedJSObject._isDefault
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 9

11 months ago
Created attachment 8935860 [details] [diff] [review]
Patch

Here is the trivial fix I suggested in comment 8. tbh I'm not completely sure this is worth fixing as the damage is probably already done (or will already be done by the time this fix reaches actual users).
Attachment #8935860 - Flags: review?(adw)
(Assignee)

Updated

11 months ago
Assignee: nobody → florian
Status: NEW → ASSIGNED

Comment 10

11 months ago
Comment on attachment 8935860 [details] [diff] [review]
Patch

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

Thanks.  I agree with your analysis about the worth of fixing this, but ultimately it's better to fix it than not.
Attachment #8935860 - Flags: review?(adw) → review+
(Reporter)

Comment 11

11 months ago
(In reply to Florian Quèze [:florian] from comment #7)
 
> > defaultSearchEngineData.loadPath    
> > jar:[profile]/extensions/langpack-en-US@firefox.mozilla.org.xpi!browser/ddg.
> > xml
> 
> Pretty unusual. This means you are using an en-US language pack to
> "translate" your Firefox into en-US.
> 
> Is this something you have installed? Is your build not an en-US one already?
> 
> If you are using an en-US language pack on an already en-US build, it's a
> case our code doesn't cover.

At some point I was using a French macOS, and the first and only time I installed Firefox I ended up getting it in French, which was fine at that time. I eventually moved to the US, and at this point it made more sense to switch my system to English (because having my brain juggle between both languages was confusing). When I did that, Firefox remained French, so I installed the language pack to switch its interface to English. I later reinstalled the entire operating system in English, and kind of forgot about the Firefox language pack. I suppose after this system reinstall I installed a fresh Firefox (therefore ended up with an en-US one) instead of taking it from my backup, but I don't really remember if this is what I actually did. And the language pack must have stayed in my Firefox profile which I am sure I recovered from my backup.

Does that make any sense?

Yes, pretty unusual situation indeed.
(Assignee)

Comment 12

11 months ago
(In reply to Guillaume from comment #11)
> (In reply to Florian Quèze [:florian] from comment #7)

> Does that make any sense?

Yes, thanks for the explanation and confirmation. It very much matches what I had guessed :-).
See Also: → bug 1371367
(Reporter)

Comment 13

11 months ago
I just removed the language pack (predictably with no effect on the actual interface language), but in about:telemetry the line you requested still reads:

defaultSearchEngineData.loadPath     jar:[profile]/extensions/langpack-en-US@firefox.mozilla.org.xpi!browser/ddg.xml

Is there anything I should do to return that to "normal"? (whatever "normal" is).
(Assignee)

Comment 14

11 months ago
(In reply to Guillaume from comment #13)
> I just removed the language pack (predictably with no effect on the actual
> interface language), but in about:telemetry the line you requested still
> reads:
> 
> defaultSearchEngineData.loadPath    
> jar:[profile]/extensions/langpack-en-US@firefox.mozilla.org.xpi!browser/ddg.
> xml
> 
> Is there anything I should do to return that to "normal"? (whatever "normal"
> is).

We record this value at the time you set your default engine. So to get back to a more normal value, you would need to set your engine to something else and then back to ddg.

"normal" here would be:
defaultSearchEngineData.loadPath jar:[app]/omni.ja!browser/ddg.xml
defaultSearchEngineData.origin   default
(Assignee)

Comment 15

11 months ago
Is there any valid reason for us to use an en-US langpack on an en-US build? This is at least the second time this causes trouble (the other known one being bug 1371367). I'm wondering if a better fix would be to just avoid loading that useless langpack.
Flags: needinfo?(l10n)
Flags: needinfo?(francesco.lodolo)

Comment 16

11 months ago
There's a use-case to install a french language pack on a French build, to test a localization update, and we might want to ship l10n updates that way at some point in the future.

There's also a use-case to install an en-US language pack on a French build, I think.

I wonder if fr-on_top_of-fr is any different thatn en-US-on_top_of-en-US are different from a code point?

Which is a long way of saying, en-US on en-US, maybe not. But ab-CD on ab-CD, totally. Would en-US actually be different?
Flags: needinfo?(l10n)
Flags: needinfo?(francesco.lodolo)
> Would en-US actually be different?

I don't know if there's anything that breaks (I didn't investigate this bug or  bug 1371367), but I believe that there shouldn't be anything special about en-US on en-US eventually. And it should not break.

If it's breaking, it's a bug and we should fix it.

Comment 18

11 months ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e64e651fa93
do not attempt to reset search settings of users with an original default engine that isn't considered a default engine (likely due to a langpack), r=adw.

Comment 19

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4e64e651fa93
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
(Assignee)

Comment 20

11 months ago
Comment on attachment 8935860 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1419941
[User impact if declined]: Users of a langpack of the same language as the built-in locale of their build will receive a one-time silent reset of their search setting when upgrading. Yes, this is a bizarre edge case, but the patch is only helpful if it reaches users quickly, not several releases later.
[Is this code covered by automated tests?]: no.
[Has the fix been verified in Nightly?]: no.
[Needs manual test from QE? If yes, steps to reproduce]: I would like QA to verify this. Seems ok to me to verify after this has reached mozilla-beta.
[List of other uplifts needed for the feature/fix]: none.
[Is the change risky?]: Low risk
[Why is the change risky/not risky?]: Simple one line test to return early instead of applying a reset for these users.
[String changes made/needed]: none.
Attachment #8935860 - Flags: approval-mozilla-beta?
Comment on attachment 8935860 [details] [diff] [review]
Patch

skip search reset in some cases, beta58+
Attachment #8935860 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 22

11 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/d7ddb4b0dc03
status-firefox58: --- → fixed
Flags: qe-verify+
Florian, related to comment 20, could you please confirm that soft reset is the expected result, for this case?
Flags: needinfo?(florian)
(Assignee)

Comment 24

11 months ago
(In reply to Adrian Florinescu [:AdrianSV] from comment #23)
> Florian, related to comment 20, could you please confirm that soft reset is
> the expected result, for this case?

Users of DDG shoudn't see a prompt, no.
Flags: needinfo?(florian)
[Environments:]
OsX 10.13.1 beta
Ubuntu 16.04 x64
Windows 10 x64


Reproduced the original bug using 57.0 en-US, installing language pack en-US, setting default search engine duckduckgo and updating to 57.0.4, whereas the search engine was reset to google and value for browser.search.reset.status was set to silent.

Verified the fix with 57.0b3 updating to 58.0b14, redoing the above steps: no engine reset and the browser.search.reset.status preference does not exist.
status-firefox58: fixed → verified
I was not able to reproduce this issue since the 58.0.1 build is available and the update from 57.0 is performed directly to the latest available build. 
I confirm that the issue is not reproducible if setting duckduckgo default search engine and update Firefox from Firefox 57.0b3 to Firefox 59.0b5.
Tests were performed under Windows 10x64, Ubuntu 16.04x64 and under macOS 10.12.6.
Status: RESOLVED → VERIFIED
status-firefox59: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.