Profiles that use non-ascii characters can't be refreshed properly
Categories
(Firefox :: Migration, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox66 | --- | unaffected |
firefox67 | + | verified |
firefox68 | --- | verified |
People
(Reporter: obotisan, Assigned: mossop)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files)
3.03 MB,
video/quicktime
|
Details | |
47 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
Affected versions
- Firefox 68.0a1
- Firefox 67.0b7
Affected platforms
- macOS 10.14
- Windows 10 x64
- Windows 7 x86
- Ubuntu 18.04 x64
Prerequisites
Open Firefox with a new profile and use non-ascii characters (https://terpconnect.umd.edu/~zben/Web/CharSet/htmlchars.html).
Steps to reproduce
- Open a few sites in different tabs.
- Go to about:support and click on the "Refresh Firefox..." button.
- And go on with the whole process.
Expected result
- Firefox is restarted and the session restore "Success!" page is properly displayed showing the following options:
- Restore all windows & tabs
- Restore only the ones you want
Actual result
- It gets stuck on the import window and if you close it, the is browser refreshed, but there isn't a "Success!" page displayed.
Regression range
- This is a regression. I can't reproduce the issue using Fx 66.0.2. I will try to find the regression range as soon as possible.
Additional notes
- Please look at the attached video.
- It gets stuck on the Import window until you close it manually. I waited 5 minutes and nothing happened.
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=5d43adb2b26ee297c8585412c0f22521ea5d7ecb&tochange=86b7743d7a65558916f5bcdeded055c3ea4a32ec
Regressed by:
86b7743d7a65 Dave Townsend — Bug 1463198: Submit telemetry whenever the downgrade UI is shown. r=froydnj, datareview=chutten
250089b69337 Dave Townsend — Bug 1455707: Detect when running an older version than previously ran with the selected profile. r=froydnj, r=mconley, r=Gijs
a5ab8fa035ce Dave Townsend — Bug 1518632: Show the first-run experience for users that get pushed to a new profile. r=Gijs, r=flod, r=stomlinson
28e889fa4322 Dave Townsend — Bug 1522934: Add telemetry data to record what happened during profile selection at startup. r=froydnj, datareview=chutten
58babf220962 Dave Townsend — Bug 1474285: Implement dedicated profiles per install. r=froydnj, r=Gijs
d3b849c06ebc Dave Townsend — Bug 1322797: Replace selectedProfile with currentProfile and fix defaultProfile. r=froydnj, r=flod
:mossop,
Your bunch of patch seems to cause the regression. Can you please look into this?
Comment 2•6 years ago
|
||
[Tracking Requested - why for this release]:
Broken refresh/reset for users on non-ascii profiles
Comment 3•6 years ago
|
||
Any errors in the browser console once the browser does start up?
Comment 4•6 years ago
|
||
Import Wizard freezes, I must close the dialog.
And Browser Console shows the following error after new browser restarted.
Error reading typed URL history: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWindowsRegKey.open]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: resource:///modules/MSMigrationUtils.jsm :: getTypedURLs :: line 660" data: no] MSMigrationUtils.jsm:710
getTypedURLs resource:///modules/MSMigrationUtils.jsm:710
get _typedURLs resource:///modules/EdgeProfileMigrator.jsm:100
get exists resource:///modules/EdgeProfileMigrator.jsm:106
getResources resource:///modules/EdgeProfileMigrator.jsm:446
filter self-hosted:318
getResources resource:///modules/EdgeProfileMigrator.jsm:446
PMB__getMaybeCachedResources resource:///modules/MigrationUtils.jsm:436
isSourceAvailable resource:///modules/MigrationUtils.jsm:415
AsyncFunctionNext self-hosted:839
Win error 2 during operation open on file C:\Users\fuku\AppData\Local\Chromium\User Data\Local State (指定されたファイルが見つかりません。
) ChromeMigrationUtils.jsm:185
getLocalState resource:///modules/ChromeMigrationUtils.jsm:185
AsyncFunctionThrow self-hosted:843
Error detecting Chrome profiles: Win error 2 during operation open on file C:\Users\fuku\AppData\Local\Chromium\User Data\Local State (指定されたファイルが見つかりません。
) ChromeProfileMigrator.jsm:143
Chrome_getSourceProfiles resource:///modules/ChromeProfileMigrator.jsm:143
AsyncFunctionThrow self-hosted:843
TypeError: resources is null MigrationUtils.jsm:216:9
Reporter | ||
Comment 5•6 years ago
|
||
(In reply to :Gijs (he/him) from comment #3)
Any errors in the browser console once the browser does start up?
Yes, there are two errors:
- Win error 2 during operation open on file C:\Users\oana.botisan\AppData\Local\Chromium\User Data\Local State (The system cannot find the file specified.
) ChromeMigrationUtils.jsm:185
getLocalState resource:///modules/ChromeMigrationUtils.jsm:185
AsyncFunctionThrow self-hosted:824
MU_spinResolve resource:///modules/MigrationUtils.jsm:679
spinResolve chrome://browser/content/migration/migration.js:71
onImportSourcePageShow chrome://browser/content/migration/migration.js:103
init chrome://browser/content/migration/migration.js:53
onload chrome://browser/content/migration/migration.xul:1
MU_showMigrationWizard resource:///modules/MigrationUtils.jsm:892
MU_asyncStartupMigrator resource:///modules/MigrationUtils.jsm:984
AsyncFunctionNext self-hosted:820
MU_spinResolve resource:///modules/MigrationUtils.jsm:679
MU_startupMigrator resource:///modules/MigrationUtils.jsm:922
MU_startupMigrator self-hosted:989
- Error detecting Chrome profiles: Win error 2 during operation open on file C:\Users\oana.botisan\AppData\Local\Chromium\User Data\Local State (The system cannot find the file specified.
) ChromeProfileMigrator.jsm:143
Chrome_getSourceProfiles resource:///modules/ChromeProfileMigrator.jsm:143
AsyncFunctionThrow self-hosted:824
MU_spinResolve resource:///modules/MigrationUtils.jsm:679
spinResolve chrome://browser/content/migration/migration.js:71
onImportSourcePageShow chrome://browser/content/migration/migration.js:103
init chrome://browser/content/migration/migration.js:53
onload chrome://browser/content/migration/migration.xul:1
MU_showMigrationWizard resource:///modules/MigrationUtils.jsm:892
MU_asyncStartupMigrator resource:///modules/MigrationUtils.jsm:984
AsyncFunctionNext self-hosted:820
MU_spinResolve resource:///modules/MigrationUtils.jsm:679
MU_startupMigrator resource:///modules/MigrationUtils.jsm:922
MU_startupMigrator self-hosted:989
Reporter | ||
Comment 6•6 years ago
|
||
Three errors, my bad. The last one is:
- TypeError: resources is null [Learn more]
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Oana Botisan, Desktop Release QA from comment #0)
Prerequisites
Open Firefox with a new profile and use non-ascii characters (https://terpconnect.umd.edu/~zben/Web/CharSet/htmlchars.html).
In the profile name or profile path?
Comment 8•6 years ago
|
||
The problem is the profile name like 'μ'.
Bug is reproduced:
- If create profile named 'μ' (i.e %AppData%pathToProfile\random.μ )
- If manually chosen c:\data\m\foo.bar and named 'μ' in profile manager.
Bug is not reproduced. Refreshing is successfully performed:
- If manually chosen (c:\data\m\foo.μ ) and named 'micro' in profile manager.
- If manually chosen c:\data\μ\foo.bar and named 'micro' in profile manager.
Assignee | ||
Comment 9•6 years ago
|
||
This fixes two bugs. The first is that when the firefox profile migrator doesn't
know which profile to migrate it attempts to fall back to another profile. I
think this was intended to be the default but in bug 1322797 I ended up making
it the current profile, which is the profile we're restoring into now. I think
at this stage the profile directory doesn't even exist so things go wrong.
Changing to use the actual default works but....
When the profile migrator UI doesn't know what profile to migrate from it uses
the default profile. So if the profile you're actually trying to restore is not
the default we'll effectively throw its data into the archive and replace it
with data from the default profile. I'm inclined to say that if the migrator
does not know what profile to migrate from it should error at that point for
safety.
Why would the profile migrator not know what profile to migrate from? Because of
a long-standing text encoding problem. In C++ profile names are encoded in UTF8.
But we try to pass them to JS through an IDL parameter of type ACString. This
does no UTF8 decoding and so JS recieves an incorrect name if the name includes
non-ascii characters and so can't find the profile.
This patch fixes the IDL parameter to AUTF8String which does the decoding
correctly and so JS gets the name correctly.
We should probably think about whether just passing the nsIToolkitProfile object
to the migrator is a better choice here.
Comment 10•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Comment 11•6 years ago
|
||
Reporter | ||
Comment 12•6 years ago
|
||
It's like Alice said in comment 8. When you use non-ascii characters in the profile name, then the issue is reproducing.
Comment 13•6 years ago
|
||
bugherder |
Comment 14•6 years ago
|
||
bugherder |
Updated•6 years ago
|
Comment 15•6 years ago
|
||
Dave, do you intend to uplift it to beta?
Assignee | ||
Comment 16•6 years ago
|
||
(In reply to Pascal Chevrel:pascalc from comment #15)
Dave, do you intend to uplift it to beta?
Yes, the fix is pretty trivial so there should be no risk to uplifting.
Reporter | ||
Comment 17•6 years ago
|
||
I verified the fix using Latest Nightly 68.0a1 on macOS 10.13, Windows 10 x64 and Ubuntu 18.04 x64. The issue is not reproducing anymore.
Assignee | ||
Comment 18•6 years ago
|
||
Comment on attachment 9056002 [details]
Bug 1541804: Make profile refresh for non-ascii named profiles work correctly. r=Gijs
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: Bug 1322797
- User impact if declined: Firefox refresh will fail for users with non-ascii characters in their names.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This is a tiny change, mainly to how we encode the test when passing across from C++ to JavaScript. Very low risk.
- String changes made/needed:
Comment 19•6 years ago
|
||
Comment on attachment 9056002 [details]
Bug 1541804: Make profile refresh for non-ascii named profiles work correctly. r=Gijs
P1, verified on nightly, minimal patch, approved for 67 beta 10, thanks.
Comment 20•6 years ago
|
||
bugherder uplift |
Updated•6 years ago
|
Updated•6 years ago
|
Reporter | ||
Comment 21•5 years ago
|
||
I verified the fix on beta 67.0b10 on Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.13. The bug is not reproducing anymore.
Description
•