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

Status: VERIFIED FIXED

(Firefox :: Migration, defect)

Description

[Vasilica Tamas, QA [:vtamas]]

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

Comment 1

[Robert Kaiser]

[Tracking Requested - why for this release]:

Comment 2

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked. Related to bug 1192032.

Comment 3

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 4

[Justin Dolske [: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

Comment 5

[Justin Dolske [:Dolske]]

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?

Comment 6

[Justin Dolske [:Dolske]]

FWIW I can run the relevant code manually in a scratchpad and get a working nsIFile for the Packages dir.

Comment 7

[Vasilica Tamas, QA [:vtamas]]

Attachment: output.txt

Tested on several Windows versions and I’ve noticed that the Packages folder does not exist under Windows 7 and Windows Vista.

Comment 8

[Justin Dolske [:Dolske]]

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.

Comment 9

[:Gijs (he/him)]

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.

Comment 10

[:Gijs (he/him)]

Attachment: 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

Comment 11

[:Gijs (he/him)]

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

Comment 12

[Matthew N. [:MattN]]

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

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/166571fc2542

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/166571fc2542

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Gijs, is this safe for uplift to Beta?

Comment 16

[:Gijs (he/him)]

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]:

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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 18

[Sylvestre Ledru [:Sylvestre]]

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.

Comment 19

[Carsten Book [:Tomcat]]

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!

Comment 20

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/7e8f1001a66b

Comment 21

[Vasilica Tamas, QA [:vtamas]]

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?

Comment 22

[Matthew N. [:MattN]]

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

Comment 23

[Vasilica Tamas, QA [:vtamas]]

I think I found the bug where it is mentioned this error: Bug 1194692