Closed Bug 1541804 Opened 5 years ago Closed 5 years ago

Profiles that use non-ascii characters can't be refreshed properly

Categories

(Firefox :: Migration, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 68
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 + verified
firefox68 --- verified

People

(Reporter: obotisan, Assigned: mossop)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached video just refresh.mov

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

  1. Open a few sites in different tabs.
  2. Go to about:support and click on the "Refresh Firefox..." button.
  3. 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.
Severity: normal → major
Has Regression Range: --- → no
Has STR: --- → yes

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?

Has Regression Range: no → yes
Flags: needinfo?(dtownsend)
Regressed by: 1322797

[Tracking Requested - why for this release]:
Broken refresh/reset for users on non-ascii profiles

Component: Session Restore → Migration

Any errors in the browser console once the browser does start up?

Flags: needinfo?(oana.botisan)

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

(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

Flags: needinfo?(oana.botisan)

Three errors, my bad. The last one is:

Priority: -- → P1

(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?

Flags: needinfo?(dtownsend) → needinfo?(oana.botisan)

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.

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.

Pushed by dtownsend@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/11089a2be3bb
Make profile refresh for non-ascii named profiles work correctly. r=Gijs
See Also: → 1542062, 1542063
Pushed by dtownsend@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8b7df8a21b43
Make profile refresh for non-ascii named profiles work correctly. r=Gijs

It's like Alice said in comment 8. When you use non-ascii characters in the profile name, then the issue is reproducing.

Flags: needinfo?(oana.botisan)
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
Assignee: nobody → dtownsend

Dave, do you intend to uplift it to beta?

Flags: qe-verify+
Flags: needinfo?(dtownsend)

(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.

Flags: needinfo?(dtownsend)

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.

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:
Attachment #9056002 - Flags: approval-mozilla-beta?

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.

Attachment #9056002 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

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.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: