[Win][Acer] The code in the search is wrongly displayed after Firefox update
Categories
(Firefox :: Distributions, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox-esr68 | 70+ | verified |
firefox67 | --- | unaffected |
firefox68 | --- | unaffected |
firefox69 | --- | unaffected |
firefox70 | + | verified |
firefox71 | --- | verified |
People
(Reporter: mberlinger, Assigned: standard8)
Details
(Keywords: regression)
Attachments
(3 files, 2 obsolete files)
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
17.22 KB,
text/plain
|
Details | |
5.63 KB,
patch
|
RyanVM
:
approval-mozilla-esr68+
|
Details | Diff | Splinter Review |
Prerequisites
-
Make sure that the "Use selected profile without asking at startup" box is checked in FF Profile Manager
-
Make sure no version of FF is installed.
-
Make sure that you clear any FF files from so you get a clean install:
rd /s /q %userprofile%\AppData\Roaming\Mozilla
rd /s /q %userprofile%\AppData\Local\Mozilla
rd /s /q %userprofile%\AppData\LocalLow\Mozilla -
acer_distribution.zip : https://www.dropbox.com/s/ud8ii2btmrvxgl2/acer_distribution.zip?dl=0
-
acer_langpacks.zip: https://www.dropbox.com/s/qo9odm3ymjzkzuo/acer_langpacks.zip?dl=0
Affected platforms
- Windows 10x64
- Windows 7x32
Steps to reproduce
- Install an older version of Beta (eg 68.0b5 32bit), but do not start it.
- Unzip the file acer_distribution.zip into the C:\Program Files (x86)\Mozilla Firefox directory.
- Launch Firefox.
- Update Firefox to latest version .
- Perform a search from the URL bar.
Expected result
- The code in the search is pc=MOZD
Actual result
- The code in the search is pc=MOZI
Additional notes
- If I’ve updated from 70.0b5 the code in the search is pc=MOZD
- This is not limited to a particular locale, I've reproduced with en-US and ar
- On RC builds the issue isn’t reproducible
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Mike can you help out here? We could get a patch into beta 12 potentially.
Comment 2•5 years ago
|
||
Setting the severity higher since we don't know if this is intended behavior or not and it involves search codes.
Comment 3•5 years ago
•
|
||
Was this working on earlier beta? We're looking at https://bugzilla.mozilla.org/show_bug.cgi?id=1574944 as a potential cause.
It went in 8 days ago.
Updated•5 years ago
|
Reporter | ||
Comment 4•5 years ago
|
||
Hello Mike,
I’ve updated from 70.0b5 to 70.0b8 and I’ve got MOZD, but when I’ve updated from 70.0b8 to 70.0b9 I’ve got MOZI instead of MOZD.
We’ve tested on 69.0b9 a while ago and we’ve got MOZD, here’s the test plan: https://docs.google.com/document/d/1L-2VYZg_aZeq8wxDKn34JTaGfnOxRkBzmrdTdkltGd4/edit#heading=h.fynafjbd5n14
Anyway another stranger scenario that I’ve met was if I’ve updated from 70.0b5 to 70.0b9 but I didn’t open the 70.0b5 and i’ve updated to 70.0b9 I’ve got MOZI and if I’ve opened the 70.0b5 and then updated to 70.0b9 I’ve got MOZD.
I’ve performed the update with pave over since using update channels will end up to beta 11.
Comment 5•5 years ago
|
||
Bugbug thinks this bug is a regression, but please revert this change in case of error.
Assignee | ||
Comment 6•5 years ago
|
||
Maria, is it possible that you could get a log of this failure for me? I can't currently reproduce, but I have some ideas.
What I need: When you create the profile, set browser.search.log
to true. Then, get the log from the browser console when you start the latest version of FF which shows the wrong code.
Assignee | ||
Comment 7•5 years ago
|
||
A few other questions as well:
- what's the
browser.search.region
preference value before & after upgrade? - Are you testing with a VPN?
- What timezone is your computer set to?
My suspicion is that this is related to the region check kicking-in after upgrade. However, that shouldn't happen, unless it didn't get set on first run, or there's some sort of issue with saving it or the value being removed.
I can currently reproduce this with a fresh profile on 70b9, but not with an upgrade. I think I know what the issue is for the new profile, but I'd like to understand why this is triggering on upgrade as, in theory, it shouldn't generally be happening.
Assignee | ||
Comment 8•5 years ago
|
||
Assignee | ||
Comment 9•5 years ago
|
||
The patch I've attached should fix the issue that I can see. I think it'll probably fix the main issue as well.
I've done a try push that is based on the latest beta:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bc290eca1f61fc096644df40686b5c5b1ed4a827
Maria, if you could try testing with that as well, that would be useful.
Assignee | ||
Updated•5 years ago
|
Comment 10•5 years ago
|
||
Hi Mark,
Maria is in PTO so I'll try to fill the details for her.
(In reply to Mark Banner (:standard8) from comment #6)
What I need: When you create the profile, set
browser.search.log
to true. Then, get the log from the browser console when you start the latest version of FF which shows the wrong code.
- Attaching the log file.
A few other questions as well:
- what's the
browser.search.region
preference value before & after upgrade?
- The browser.search.region has the value of RO
- Are you testing with a VPN?
- No VPN was used for this testing.
- What timezone is your computer set to?
- UTC +02:00
I can currently reproduce this with a fresh profile on 70b9, but not with an upgrade. I think I know what the issue is for the new profile, but I'd like to understand why this is triggering on upgrade as, in theory, it shouldn't generally be happening.
-
I have managed to reproduce this by both triggering the update or upgrade and by just installing 70.0b9 directly as well via zip and installer (without performing an update or upgrade) inside the C:\Program Files\Mozilla Firefox path
-
I can't reproduce this on the provided try build ( from comment 9) with a note that I can't perform a pave over since only the zip format seems to work.
Updated•5 years ago
|
Comment 11•5 years ago
|
||
Adding the correct log file.
Assignee | ||
Comment 12•5 years ago
|
||
Thank you for the additional info. That makes me pretty sure this is the same issue as what I found, so I'm going to go ahead and get this landed.
Assignee | ||
Updated•5 years ago
|
Comment 13•5 years ago
|
||
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d31bf4ac2cb6 Ensure that the search service doesn't load built-in engines over the top of distribution engines. r=mikedeboer
Comment 14•5 years ago
|
||
bugherder |
Assignee | ||
Comment 15•5 years ago
|
||
Comment on attachment 9098396 [details]
Bug 1585367 - Ensure that the search service doesn't load built-in engines over the top of distribution engines.
Beta/Release Uplift Approval Request
- User impact if declined: Probably no significant impact on users. However, a user with a distribution pack can get into the situation where we're providing the wrong codes for the search engine (the Firefox ones, rather than the distribution ones).
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: See comment 0
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Small change that only affects preventing distribution engines from having their settings overridden.
- String changes made/needed: None
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 16•5 years ago
|
||
Hi, Can someone please reupload the files we need to retest this issue, the acer distribution file returns a 404.
Comment 18•5 years ago
|
||
Rares or Maria, if you can verify tomorrow that will be in time for the beta 14 build :) Thanks!
Reporter | ||
Comment 19•5 years ago
|
||
Hello,
This is verified fixed using Firefox 71.0a1 (20191008214557) on Windows 10x64 and Windows 7x32.
Comment 20•5 years ago
|
||
Comment on attachment 9098396 [details]
Bug 1585367 - Ensure that the search service doesn't load built-in engines over the top of distribution engines.
Fix for search issue, verified in nightly, OK for beta 14 uplift.
Comment 21•5 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 22•5 years ago
|
||
This is also verified fixed using Firefox 70.0b14 on Windows 10x64 and Windows 7x32.
Assignee | ||
Comment 23•5 years ago
|
||
Comment on attachment 9098396 [details]
Bug 1585367 - Ensure that the search service doesn't load built-in engines over the top of distribution engines.
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed for uplift of bug 1574944
- User impact if declined: Little impact.
- Fix Landed on Version: 70
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Includes automated tests for this scenario
- String or UUID changes made by this patch: None
Assignee | ||
Comment 24•5 years ago
|
||
Mike, I've been trying this patch out in combination with the patch I've just attached to bug 1574944.
The strange thing is, the newly added test toolkit/components/search/tests/xpcshell/test_maybereloadengine_update.js
doesn't fail with the SearchService.jsm part commented out (as per this version of the patch).
This makes me concerned if a) there's some other patch we're missing, b) if this isn't an issue on ESR even with bug 1574944 fixed, c) something else.
What I haven't had time to do yet, is to run both patches with the distribution set up as mentioned in this bug - when I ran them before, I copied the extracted distribution directory into objdir-ff/dist/Nightly.app/Contents/Resources
and tested like that.
If that still works with bug 1574944 applied, and without this applied, then maybe for some reason that is ok to ship like that... just strange and I wonder what changed.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 25•5 years ago
|
||
Comment on attachment 9098396 [details]
Bug 1585367 - Ensure that the search service doesn't load built-in engines over the top of distribution engines.
Moving request to new patch.
Assignee | ||
Comment 26•5 years ago
|
||
Please see comment 1585367.
Manual testing of the patch shows that it correctly fixes the issue here on ESR after bug 1574944 is applied as well.
The automatic test doesn't actually detect the issue for ESR. I suspect this is something to do with changed setup requirements between release and ESR, but I haven't debugged what yet. I don't think this is enough to delay shipping though.
Updated•5 years ago
|
Comment 27•5 years ago
|
||
Comment on attachment 9101580 [details] [diff] [review] ESR Patch Fixes a regression from bug 1574944. Approved for 68.2esr.
Updated•5 years ago
|
Comment 28•5 years ago
|
||
bugherder uplift |
Assignee | ||
Comment 29•5 years ago
|
||
I just pushed a test-only fix to mozilla-release as the original uplift (comment 21) missed the fact that the test added to a differently named xpcshell ini file that isn't present in 70.
https://hg.mozilla.org/releases/mozilla-release/rev/cfcae5cd47646c9b9f58d25d228987ec4f7553d4
No bad consequences, just the test wasn't being run.
Reporter | ||
Comment 30•5 years ago
|
||
This is also verified fixed using Firefox 68.2.0esr on Windows 10x64 and Windows 7x32.
Updated•2 years ago
|
Description
•