413021 - Migration/Import Wizard offer migrate from IE on Mac (where no IE is installed) ?

Status: VERIFIED FIXED

(Firefox :: Migration, defect, P2)

Description

[Carsten Book [:Tomcat]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011812 Firefox/3.0b3pre

Reproduced with a Debug Build and Trunk Nightly build from ftp.m.o on Mac 10.4

Steps to reproduce:
Rename or delete your Firefox Profile folder, so that the Migration Wizard comes up.

Check the Migration Wizard..you see that Microsoft Internet Explorer is available ..also when no IE is installed on the System.

Comment 1

[Carsten Book [:Tomcat]]

Attachment: Migration Wizard on mac

Comment 2

[Carsten Book [:Tomcat]]

Attachment: After Safari Migration

Even more worse is (and i think a reason for blocking for this bug) that the Folder of imported Bookmarks from Safari are called "Imported IE favorites"

Comment 3

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

That doesn't happen for me with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011804 Minefield/3.0b3pre ID:2008011804

The imported bookmarks from Safari are placed directly inside the bookmarks menu. Is this like expected?

Comment 4

[Tracy Walker [:tracy]]

Fresh default 10.4 machine has a Directory in Users/<user>/Library/Preferences/Explorer/Favorites.html

remove/rename the Explorer directory, then this bug goes away. 

Odd thing is that Favorites.html is a list of Apple bookmarks.  Why is it there? IE has never been installed or used on these test boxes.

Comment 5

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

Tracy, this folder doesn't exist on my 10.4.11 installation. It's already running about a year now. So you really installed no other software which has placed this file there?

Comment 6

[Tracy Walker [:tracy]]

Yes,  I double checked this on some minis that we had just recently installed 10.4 on. Those machines have had no additional software added beyond a default installation except a vnc server.  I'm quite puzzled where that directory/file is coming from. I'm going to find a machine in MV to "start from the beginning" to see if I can detect when and why the suspect file appears.

Comment 7

[Carsten Book [:Tomcat]]

Drivers: Requesting blocking for this. This is a kind of confusing User Experience and even there is a Imported IE Favorites Folder on Mac, see comment #2. 

More Important is maybe that this problem affects not only the Migration, also the Import Wizard shows the same.

Comment 8

[Dietrich Ayala (:dietrich)]

(In reply to comment #3)
> That doesn't happen for me with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4;
> en-US; rv:1.9b3pre) Gecko/2008011804 Minefield/3.0b3pre ID:2008011804
> 
> The imported bookmarks from Safari are placed directly inside the bookmarks
> menu. Is this like expected?
> 

Henrik: are you saying that it doesn't happen on build id 2008011804, but you *are* able to reproduce with build id 2008011812 (the reporter's build id)?

I don't think this should block beta 3, but should block final.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

I agree with Dietrich; blocker, but not for Beta 3. Can we get a clear statement of when this happens? So far I've seen:

 - happens on clean install of 10.4
 - doesn't happen on a clean install of 10.4.11
 - bookmarks from Safari import go into bookmarks menu, not folder within bookmarks menu (is that expected?)

Comment 10

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

Dietrich, sorry but I don't have such a folder on my system. So it won't be visible with any build. 

Comment 11

[Michael Schonfeld]

It seems to me that the problem lies in the fact that we check for the existence of 'Favorites.html'.  Apparently, other programs like Microsoft office for Mac, and who-knows-what uses the same file, which creates a confusion for the Firefox migration wizard. What more, is that we are unsure as to why this file is created without IE.

I believe that the correct way to check for the presence of IE on Mac OS, is to check com.apple.internetconfig (using nsInternetConfigServices) for the existence of the '4D534945•WWWHomePage' node.  Evidently, somewhere along the line, Microsoft had decided to move *all* of IEs preferences into the internetconfig file, preceding all pref keys with '4D534945•'.

Comment 12

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

This bug has to be introduced by one of the Apple system updates later then 10.4.8 or so. I created a fresh user account the last days which shows exactly the same issue now. My old profile which was created February last year isn't affected.

Comment 13

[Michael Schonfeld]

Henrik: Can you check, in your old profile - the one that's not affected by this syndrome, if your com.apple.internetconfig.plist (~/Library/Preferences) has those IE keys in it ('4D534945•SOME_KEY_HERE')? They are all stored under 'ic-added'.   If they are present in your old profile, I see no reason why we should add this *extra* check, before checking for Favorites.html in order to make sure IE is really installed.

Comment 14

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

I dig into this problem and have to withdraw my last comment. It doesn't rely on the installed OS X version. The migration wizard is raised when you start Firefox for the first time within a new user account. In that case the folder ~/Library/Application Support/Firefox doesn't exist. Any further creation of a new profile misses the migration wizard. You can reproduce it after deleting the folder mentioned before.

Comment 15

[Michael Schonfeld]

Attachment: v1

The way I see it is,
1. The file we are currently using to determine whether IE is installed, Favorites.html, is very misleading, because various different programs use it.
2. IE creates a whole bunch of pref keys in com.apple.internetconfig to store all of its preferences.  By simply checking one of its preferences, we can determine whether IE is really installed or not.

Comment 16

[Mano (::mano, needinfo? for any questions; not reading general bugmail)]

Comment on attachment 306716 [details] [diff] [review]
v1

reviewed over IM.

Comment 17

[Michael Schonfeld]

Attachment: v1.2

Thanks for the review Asaf!

Comment 18

[Mano (::mano, needinfo? for any questions; not reading general bugmail)]

Comment on attachment 306717 [details] [diff] [review]
v1.2

>Index: browser/components/migration/src/nsMacIEProfileMigrator.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/migration/src/nsMacIEProfileMigrator.cpp,v
>retrieving revision 1.16
>diff -u -p -8 -r1.16 nsMacIEProfileMigrator.cpp
>--- browser/components/migration/src/nsMacIEProfileMigrator.cpp	30 Aug 2007 03:01:07 -0000	1.16
>+++ browser/components/migration/src/nsMacIEProfileMigrator.cpp	1 Mar 2008 13:20:59 -0000
>@@ -43,19 +43,21 @@
> #include "nsIObserverService.h"
> #include "nsIProfileMigrator.h"
> #include "nsIServiceManager.h"
> #include "nsIStringBundle.h"
> #include "nsISupportsArray.h"
> #include "nsISupportsPrimitives.h"
> #include "nsServiceManagerUtils.h"
> #include "nsIProperties.h"
>+#include <InternetConfig.h>
> 
> #define MACIE_BOOKMARKS_FILE_NAME NS_LITERAL_STRING("Favorites.html")
> #define MACIE_PREFERENCES_FOLDER_NAME NS_LITERAL_STRING("Explorer")
>+#define MACIE_DEFAULT_HOMEPAGE_PREF  "\p4D534945�WWWHomePage"
> #define TEMP_BOOKMARKS_FILE_NAME NS_LITERAL_STRING("bookmarks_tmp.html")
> 
> #define MIGRATION_BUNDLE "chrome://browser/locale/migration/migration.properties"
> 
> ///////////////////////////////////////////////////////////////////////////////
> // nsMacIEProfileMigrator
> 
> NS_IMPL_ISUPPORTS1(nsMacIEProfileMigrator, nsIBrowserProfileMigrator)
>@@ -132,21 +134,46 @@ nsMacIEProfileMigrator::GetMigrateData(c
>                           aReplace, mSourceProfile, aResult);
> 
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
> nsMacIEProfileMigrator::GetSourceExists(PRBool* aResult)
> {
>+  // Since the IE bookmarks file can sometimes be created by programs
>+  // other than Internet Explorer, thus misleading, we must first
>+  // check whether IE is even installed on this Mac.  We accomplish this by
>+  // checking one of IEs stored preferences in the apple.internetconfig file.
>+  PRBool prefExists = PR_FALSE;
>+  OSErr err;
>+  ICInstance icInstance;
>+
>+  err = ::ICStart(&icInstance, 'XPCM');

Make that 'FRFX', for parity with the other migrator.

>+  if (err == noErr) {
>+    ICAttr attrs;
>+    Str255 IEhomePageValue;
>+    long size = kICFileSpecHeaderSize;
>+    err = ::ICGetPref(icInstance, MACIE_DEFAULT_HOMEPAGE_PREF, &attrs,
>+                      IEhomePageValue, &size);
>+    if (err == noErr)
>+      prefExists = PR_TRUE;
>+  }
>+  ::ICStop(icInstance);
>+

I believe you should call ICStop only if ICStart succeeded.
 
Looks good otherwise, r=mano.

Comment 19

[Mano (::mano, needinfo? for any questions; not reading general bugmail)]

Comment 2 looks like a separate bug, btw.

Comment 20

[Michael Schonfeld]

Attachment: for checkin

Thanks a bunch Asaf!

Comment 21

[Mano (::mano, needinfo? for any questions; not reading general bugmail)]

...which I couldn't reproduce, I guess you chose to import From IE first, at which point the "From IE" folder was created.

Comment 22

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

Comment on attachment 306721 [details] [diff] [review]
for checkin

>Index: browser/components/migration/src/nsMacIEProfileMigrator.cpp
>===================================================================
> #define MACIE_PREFERENCES_FOLDER_NAME NS_LITERAL_STRING("Explorer")
>+#define MACIE_DEFAULT_HOMEPAGE_PREF  "\p4D534945�WWWHomePage"

No need for a NS_LITERAL_STRING here?

Comment 23

[Mano (::mano, needinfo? for any questions; not reading general bugmail)]

No, this string is used for a system call.

Comment 24

[Mano (::mano, needinfo? for any questions; not reading general bugmail)]

mozilla/browser/components/migration/src/nsMacIEProfileMigrator.cpp 1.17

Comment 25

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko Minefield/3.0b4pre ID:2008030115. The migration wizard doesn't appear anymore. 

Comment 26

[Michael Schonfeld]

Henrik: Do you mean it doesn't appear at all, or that Internet Explorer is no longer listed as an import option?

Comment 27

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

I'm not able to test the migration from IE because I don't have it installed. But for a new OS X user the migration wizard doesn't appear until he has started Safari for at least one time. That means we don't fail anymore in detecting a non-installed IE.

Comment 28

[Michael Schonfeld]

Perfect. Thanks Henrik!

Comment 29

[Reed Loden [:reed]]

Comment on attachment 306721 [details] [diff] [review]
for checkin

This completely landed without approval. Please see the top of tinderbox or
http://wiki.mozilla.org/TreeStatus before landing things without approval.

Comment 30

[Mano (::mano, needinfo? for any questions; not reading general bugmail)]

Backed out.

Comment 31

[Mike Beltzner [:beltzner, not reading bugmail]]

This doesn't block beta 4. I'm going to approve the patch, though, as it looks safe enough and will make for a less confusing migration experience.

Comment 32

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 306721 [details] [diff] [review]
for checkin

a1.9b4=beltzner

Comment 33

[Reed Loden [:reed]]

Checking in browser/components/migration/src/nsMacIEProfileMigrator.cpp;
/cvsroot/mozilla/browser/components/migration/src/nsMacIEProfileMigrator.cpp,v  <--  nsMacIEProfileMigrator.cpp
new revision: 1.19; previous revision: 1.18
done

Comment 34

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko Minefield/3.0b5pre ID:2008030618

I have to mention that comment 2 is still not fixed! Tomcat, I think you should file a new bug about this remaining issue. It will really confuse people when importing their Safari bookmarks.

Comment 35

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

(In reply to comment #34)
> I have to mention that comment 2 is still not fixed! Tomcat, I think you should
> file a new bug about this remaining issue. It will really confuse people when
> importing their Safari bookmarks.

This is covered by bug 400193.