346602 - crash when importing favorites from Yahoo browser [@ nsIEProfileMigrator::GetSourceHomePageURL]

Status: VERIFIED FIXED

(Firefox :: Migration, defect)

Description

[Adam Guthrie]

I saw this report when browsing Talkback. Maybe we should be checking the result of NS_NewURI here? 
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/components/migration/src/nsIEProfileMigrator.cpp&mark=518&rev=MOZILLA_1_8_0_BRANCH#495

Incident ID: 21578032
Stack Signature	nsIEProfileMigrator::GetSourceHomePageURL bb7729d3
Product ID	Firefox15
Build ID	2006071912
Trigger Time	2006-07-29 19:50:53.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	firefox.exe + (004b88d4)
URL visited	
User Comments	i was jus importing the favorties from ie n it failed everytime i go file import it does that dont kno y. please help n also how cn i tranfser al my fav from yahoo browser to fire fox??
Since Last Crash	132 sec
Total Uptime	733 sec
Trigger Reason	Access violation
Source File, Line No.	c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/browser/components/migration/src/nsIEProfileMigrator.cpp, line 518
Stack Trace 	
nsIEProfileMigrator::GetSourceHomePageURL  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/browser/components/migration/src/nsIEProfileMigrator.cpp, line 518]
XPTC_InvokeByIndex  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp, line 102]
XPCWrappedNative::CallMethod  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 2155]
XPC_WN_GetterSetter  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1477]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1188]
js_InternalInvoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1285]
js_InternalGetOrSet  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1344]
js_GetProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 3085]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 5335]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
fun_apply  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 1669]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1188]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3584]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
js_InternalInvoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1285]
js_InternalGetOrSet  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1344]
js_SetProperty  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 3171]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3419]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
js_Interpret  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 3584]
js_Invoke  [c:/builds/tinderbox/Fx-Mozilla1.8.0-Release/WINNT_5.2_Depend/mozilla/js/src/jsinterp.c, line 1208]
...

Comment 1

[Adam Guthrie]

It was pointed out to me that "Yahoo browser" could be the IE7 beta that Yahoo re-branded.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #0)
> I saw this report when browsing Talkback. Maybe we should be checking the
> result of NS_NewURI here?

Yes, we should!

Comment 3

[Adam Guthrie]

Attachment: So something like this?

Comment 4

[Michael Wu [:mwu]]

(In reply to comment #3)
> Created an attachment (id=231363) [edit]
> So something like this?
> 
Checking homePageURI for null may be enough.

Comment 5

[Adam Guthrie]

There've been 500 crashes like this since 2006-07-24 (11 today). I'm not quite sure why this isn't showing up in top crashes. Judging from the comments, this happens with both IE6 and IE7.

Anyway, we need to not break people when they try to import their homepage from IE. My patch fixes the crash, but we still won't import someone's homepage correctly. We need to figure out what a failing homepage looks like. 

For those trying to reproduce: in IE, set your homepage to different web sites and then importing your bookmarks via File > Import. If you crash, look in the Windows registry and see what the value of "Software\\Microsoft\\Internet Explorer\\Main" is in the HKEY_CURRENT_USER tree.

Comment 6

[Carsten Book [:Tomcat]]

Done some research on this Yahoo Browser. It seems this Browser is not free for all users to download, its seems this a special edition for SBC/ AT&T Customers -> http://promo.yahoo.com/att/mem/browser/ System Requirements:* SBC Yahoo! DSL.

Try to find another download location for this Browser and also will run later some testruns with IE 7

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 231363 [details] [diff] [review]
So something like this?

null checks are good.  This should only need a day of baking on trunk, this is pretty straightforward.

Comment 8

[Martijn Wargers (dead)]

Checking in nsIEProfileMigrator.cpp;
/cvsroot/mozilla/browser/components/migration/src/nsIEProfileMigrator.cpp,v  <--
  nsIEProfileMigrator.cpp
new revision: 1.43; previous revision: 1.42
done

Checked in on trunk.

Comment 9

[Carsten Book [:Tomcat]]

(In reply to comment #5)
> For those trying to reproduce: in IE, set your homepage to different web sites
> and then importing your bookmarks via File > Import. If you crash, look in the
> Windows registry and see what the value of "Software\\Microsoft\\Internet
> Explorer\\Main" is in the HKEY_CURRENT_USER tree.
> 

Tested with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.7) Gecko/20060831 Firefox/1.5.0.7 and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060829 BonEcho/2.0b2 with an Internet Explorer 7 Beta 7.0.5700.6

Registry under HKEY_CURRENT_USER\Software\Microsoft\Internet Explorer\Main was updated when changed Start Page. Also with Firefox 1.5.0.7 and Firefox 2 Beta 2 no crash on import

Comment 10

[Adam Guthrie]

Comment on attachment 231363 [details] [diff] [review]
So something like this?

This patch fixes a crash when people are importing their data from IE.

Comment 11

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

Comment on attachment 231363 [details] [diff] [review]
So something like this?

a=beltzner on behalf of 181drivers

Comment 12

[Adam Guthrie]

Comment on attachment 231363 [details] [diff] [review]
So something like this?

There are actually quite a bit of Talkback reports for this crash in 1.5.0.x. We should get this into the 1.8.0 branch, too.

Comment 13

[Robert Sayre]

Checking in nsIEProfileMigrator.cpp;
/cvsroot/mozilla/browser/components/migration/src/nsIEProfileMigrator.cpp,v  <--  nsIEProfileMigrator.cpp
new revision: 1.29.4.11; previous revision: 1.29.4.10
done

Comment 14

[Jungshik Shin]

The patch here may be just wall-papering a more serious issue. Not that I'm against this patch but that I think we have to figure out why NS_NewURI fails. The latest patch for bug 250255 might fix it.

Comment 15

[Jungshik Shin]

sorry for bug spam

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 231363 [details] [diff] [review]
So something like this?

approved for 1.8.0 branch, a=dveditz for drivers

Comment 17

[Martijn Wargers (dead)]

Checking in nsIEProfileMigrator.cpp;
/cvsroot/mozilla/browser/components/migration/src/nsIEProfileMigrator.cpp,v  <--
  nsIEProfileMigrator.cpp
new revision: 1.29.4.1.2.2; previous revision: 1.29.4.1.2.1
done

Checked in on the 1.8.0.x branch.

Comment 18

[juan becerra [:juanb]]

I'm unable to reproduce this crash with 1505, 1507 when importing bookmarks from IE7 according the steps in comment #5. The homepages (different websites in different tabs) are not imported.

Comment 19

[Adam Guthrie]

We'll have to verify this based on Talkback reports. There've been no crashes in Firefox 2 with this stacktrace, so marking verified1.8.1. We'll have to wait a bit to see if it's fixed in 1.8.0.9.