Closed Bug 413021 Opened 13 years ago Closed 13 years ago

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

Categories

(Firefox :: Migration, defect, P2)

Product:
Firefox
Component:
Migration
Platform:
All
macOS
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 3 beta4

People

(Reporter: cbook, Assigned: dev)

Details

Attachments

(3 files, 2 obsolete files)

Migration Wizard on mac
32.84 KB, image/png
Details
After Safari Migration
27.08 KB, image/png
Details
for checkin
2.56 KB, patch
beltzner
: approval1.9b4+
Details | Diff | Splinter Review 
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.
Flags: blocking-firefox3?
Attached image 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"
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?
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.
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?
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.
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.
Summary: Migration Wizard offer migrate from IE on Mac (where no IE is installed) ? → Migration/Import Wizard offer migrate from IE on Mac (where no IE is installed) ?
Target Milestone: --- → Firefox 3 beta3
(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.
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?)
Flags: blocking-firefox3? → blocking-firefox3+
Keywords: relnote
Priority: -- → P2
Target Milestone: Firefox 3 beta3 → Firefox 3 beta4
Dietrich, sorry but I don't have such a folder on my system. So it won't be visible with any build.
Target Milestone: Firefox 3 beta4 → Firefox 3
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•'.
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.
Hardware: PC → All
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.
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.
Attached patch v1 (obsolete) — — Splinter Review 
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.
Assignee: nobody → dev
Status: NEW → ASSIGNED
Attachment #306716 - Flags: review?(mano)
Attached patch v1.2 (obsolete) — — Splinter Review 
Thanks for the review Asaf!
Attachment #306716 - Attachment is obsolete: true
Attachment #306717 - Flags: review?(mano)
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.
Attachment #306717 - Flags: review?(mano) → review+
Attached patch for checkin — — Splinter Review 
Thanks a bunch Asaf!
Attachment #306717 - Attachment is obsolete: true
...which I couldn't reproduce, I guess you chose to import From IE first, at which point the "From IE" folder was created.
Assignee: dev → nobody
Status: ASSIGNED → NEW
Component: Places → Migration
QA Contact: places → migration
Target Milestone: Firefox 3 → Firefox 3 beta4
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?
No, this string is used for a system call.
mozilla/browser/components/migration/src/nsMacIEProfileMigrator.cpp 1.17
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
Henrik: Do you mean it doesn't appear at all, or that Internet Explorer is no longer listed as an import option?
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.
Perfect. Thanks Henrik!
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.
Attachment #306721 - Flags: approval1.9b4?
Backed out.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
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.
Target Milestone: Firefox 3 beta4 → Firefox 3
Comment on attachment 306721 [details] [diff] [review]
for checkin

a1.9b4=beltzner
Attachment #306721 - Flags: approval1.9b4? → approval1.9b4+
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
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Keywords: relnote
Resolution: --- → FIXED
Target Milestone: Firefox 3 → Firefox 3 beta4
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.
Status: RESOLVED → VERIFIED
(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.
You need to log in before you can comment on or make changes to this bug.