[Windows] Exception trying to find the Edge favorites directory when importing data from other browsers

VERIFIED FIXED in Firefox 42

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: vasilica.mihasca, Assigned: Gijs)

Tracking

({regression})

41 Branch
Firefox 43
All
Windows
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox41+ wontfix, firefox42+ verified, firefox43 verified)

Details

Attachments

(2 attachments)

Reproducible on: Firefox 41 Beta 6 using Windows 7 64-bit

STR
1.Run Firefox for the first time or remove the profiles folder.
2.Select a browser from the "Import Settings and Data" window and click on “Next” button.
3.Select “Import your home page from...” ( for Internet Explorer and Safari) 
4.Press "Next" and then “Finish” buttons.
4.Check the Browser Console.

ER
The data is successfully imported and no errors are thrown in Browser Console.

AR
The following error is thrown in console:

Exception trying to find the Edge favorites directory: [Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFile.directoryEntries]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource:///modules/MSMigrationUtils.jsm :: getEdgeLocalDataFolder :: line 153"  data: no] MSMigrationUtils.jsm:164:0

Additional notes:
- This issue is reproducible on Firefox 41 Beta 6 under Windows 7 64-bit.
- This issue does not reproduce on Ubuntu and Mac platforms.


This issue is not reproducible in Firefox 41 Beta 5 and I think that was regressed by Bug 1192032
[Tracking Requested - why for this release]:
Tracked. Related to bug 1192032.
Depends on: 1192032
Justin, QE team found this bug while verifying the fixes landed in 41.0b6 around importing cookies, favorites from Edge browser. Would you be able to help find an owner to investigate/fix? We are about a week away from gtb 41 RC and therefore the slight urgency to investigate and uplift patch from a Beta41 POV.
Flags: needinfo?(dolske)
This code came in with bug 1192036, and there was some related discussion in bug 1193387 comment 17 about uncertainty about getting some path bits right.

But that packages.directoryEntries is throwing isn't good. Seems surprising that your Packages dir would be empty(?). Can you run the following in a command prompt and attach the output?

  tree /a "%LOCALAPPDATA%\Packages" > output.txt
Blocks: 1192036
Flags: needinfo?(dolske) → needinfo?(vasilica.mihasca)
Actually, an empty dir shouldn't cause this, I'd expect that code to just return an empty enumerator. So that implies %APPLOCALDATA%\Packages itself doesn't exist? That would be even more surprising. I wonder if there's a character encoding bug or something like that happening here?

Jared, any ideas?
Flags: needinfo?(jaws)
FWIW I can run the relevant code manually in a scratchpad and get a working nsIFile for the Packages dir.
Posted file output.txt
Tested on several Windows versions and I’ve noticed that the Packages folder does not exist under Windows 7 and Windows Vista.
Flags: needinfo?(vasilica.mihasca)
Oh! I didn't notice this on on Win7. Obviously Edge isn't there, but I'd expect IE data to be migrated.

Looks like none of this code expects to be running unless the Edge migration is running, so that this code is running is a bug.
Flags: needinfo?(gijskruitbosch+bugs)
Just so we're clear, the IE data is still correctly imported here. The only issue is that the exception gets reported to the browser console. That's not ideal, and we can probably guard against Edge being considered here, but the migrator code is robust enough that the current situation does not cause problems on non-win10 machines where this directory doesn't exist.
Bug 1200598 - fix exception thrown when using migration on win8.1 or below, r?MattN
Attachment #8658205 - Flags: review?(MattN+bmo)
(In reply to :Gijs Kruitbosch from comment #10)
> Created attachment 8658205 [details]
> MozReview Request: Bug 1200598 - fix exception thrown when using migration
> on win8.1 or below, r?MattN
> 
> Bug 1200598 - fix exception thrown when using migration on win8.1 or below,
> r?MattN

This implements the suggestion from bug 1192035 comment 30. I've verified that this fixes the issue.
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8658205 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8658205 [details]
MozReview Request: Bug 1200598 - fix exception thrown when using migration on win8.1 or below, r?MattN

https://reviewboard.mozilla.org/r/18503/#review16559
Flags: needinfo?(jaws)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/166571fc2542
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Gijs, is this safe for uplift to Beta?
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8658205 [details]
MozReview Request: Bug 1200598 - fix exception thrown when using migration on win8.1 or below, r?MattN

(In reply to Ritu Kothari (:ritu) from comment #15)
> Gijs, is this safe for uplift to Beta?

I think so, but I don't think it *needs* uplift to beta, so I don't think we should do that (esp. now that it's so late in the cycle). The error in the browser console is harmless, and few people are going to open it for their migration. It probably wants uplift to 42.

Approval Request Comment
[Feature/regressing bug #]: bug 1192032 et al.
[User impact if declined]: an error shows up in the browser console when migrating from another browser on win7 and below, maybe win8. The error has no functional consequences on the migration.
[Describe test coverage new/current, TreeHerder]: no.
[Risks and why]: low risk, avoids running the code that throws errors on win8 and below.
[String/UUID change made/needed]:
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8658205 - Flags: approval-mozilla-aurora?
Gijs does not feel the need to uplift this to Beta41 and given comment 9, this is not a release blocker and a wontfix for 41 now.
Comment on attachment 8658205 [details]
MozReview Request: Bug 1200598 - fix exception thrown when using migration on win8.1 or below, r?MattN

OK, exceptions are ugly, taking it for 42.
Attachment #8658205 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Hi, this cause conflicts when uplifting:

grafting 297788:166571fc2542 "Bug 1200598 - fix exception thrown when using migration on win8.1 or below, r=MattN"
merging browser/components/migration/EdgeProfileMigrator.js
3 files to edit
merging browser/components/migration/EdgeProfileMigrator.js failed!
abort: unresolved conflicts, can't continue

can you take a look here, thanks!
Flags: needinfo?(gijskruitbosch+bugs)
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/7e8f1001a66b
Flags: needinfo?(gijskruitbosch+bugs)
I was able to reproduce this issue on Firefox 43.0a1 (2015-09-08) under Windows 7 64-bit.

Verified fixed on Firefox 43.0a1 (2015-09-20) and Firefox 42.0a2 (2015-09-20) using Windows 7 64-bit. The Edge Exception is no longer thrown in browser console.

I have noticed another error while migrating from Internet Explorer. This issue is also thrown in browser console on Firefox 43.0a1 from 2015-08-21. But, despite this error, the Internet Explorer cookies are successfully imported.

Unable to migrate cookie: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsICookieManager2.add]"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: resource:///modules/MSMigrationUtils.jsm :: _parseCookieBuffer :: line 691"  data: no] MSMigrationUtils.jsm:636:0


Should I file a new bug for this error or this is already tracked?
Status: RESOLVED → VERIFIED
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Vasilica Mihasca, QA [:vasilica_mihasca] from comment #21)
> I have noticed another error while migrating from Internet Explorer. This
> issue is also thrown in browser console on Firefox 43.0a1 from 2015-08-21.
> But, despite this error, the Internet Explorer cookies are successfully
> imported.

Are you sure every single one is imported? This error would lead me to believe that at least one didn't import.

> Unable to migrate cookie: [Exception... "Component returned failure code:
> 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsICookieManager2.add]"  nsresult:
> "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame ::
> resource:///modules/MSMigrationUtils.jsm :: _parseCookieBuffer :: line 691" 
> data: no] MSMigrationUtils.jsm:636:0
> 
> Should I file a new bug for this error or this is already tracked?

I don't see any mention of this bug at https://wiki.mozilla.org/QA/Firefox_migrators#Edge_migrator so please file it so we can investigate which cookie is missing.
Flags: needinfo?(gijskruitbosch+bugs)
I think I found the bug where it is mentioned this error: Bug 1194692
You need to log in before you can comment on or make changes to this bug.