321886 - Crash when importing bookmarks from Firefox

Status: VERIFIED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[Dan Villiom Podlaski Christiansen]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

I use Firefox 1.5, and I wanted to give Camino 1.0b2 a try. However, it crashed when I attempted to import my bookmarks from Firefox.

Reproducible: Always

Steps to Reproduce:
1. Launch Camino.
2. File -> Import bookmarks... -> Firefox

Actual Results:  
Camino crashes.

Expected Results:  
Bookmarks are imported. Camino does not crash.

Comment 1

[Dan Villiom Podlaski Christiansen]

Attachment: Crash log

This is the crash reporter output. Please tell me if you need further debug info, for instance from GDB.

Comment 2

[Dan Villiom Podlaski Christiansen]

I was about to attach my Firefox bookmarks.html to the bug, but I'd rather not upload my bookmarks to a public site like this. I wouldn't mind mailing it to anyone if it's useful for fixing the bug, so please tell me if you want it.

Comment 3

[Mike Pinkerton (not reading bugmail)]

please email to mikepinkerton at mac.com. Thanks!

Comment 4

[Mike Pinkerton (not reading bugmail)]

here's the problem

<A ADD_DATE="1094590064" LAST_MODIFIED="1135952017" FEEDURL="http://news.bbc.co.uk/rss/newsonline_world_edition/front_page/rss091.xml" ID="rdf:#$ZOE8g">BBC News Online</A>

we're assuming that there's an HREF in all anchors and ignoring the fact thatthere might not be one. This used to be ok, until we added the code to check for a menu spacer (bug 309008). Both smfr and i missed this. We need a check for |isAtEnd| before checking the token string for the spacer.

Comment 5

[Mike Pinkerton (not reading bugmail)]

Attachment: patch

this fixes it for the bookmarks set that crashed it.

the logic in this area isn't the greatest, and while we could clean it up with some better high-level grouping, i think re-jiggering and re-indenting the if-statements here  is overkill. Smfr, what do you think?

Comment 6

[Simon Fraser [no longer active]]

Comment on attachment 207184 [details] [diff] [review]
patch

I wish we could use Gecko to parse the HTML to a DOM and use that.

Comment 7

[Mike Pinkerton (not reading bugmail)]

fixed on trunk and both branches.

Comment 8

[Mike Pinkerton (not reading bugmail)]

dan, would it be possible to pull tomorrow's (12/31) nightly build (or any date after that) and verify this fix?

Comment 9

[Dan Villiom Podlaski Christiansen]

Of course, I was planning to do so anyway :)

Comment 10

[Samuel Sidler (old account; do not CC)]

*** Bug 321970 has been marked as a duplicate of this bug. ***

Comment 11

[John Spooner]

I eventually got Import to work, but I think you should be aware:

3 reasons (minor bugs) I THOUGHT my Firefox bookmarks were not importing:

1) Blue "Import" button in window did not react after clicking -- i.e. button did not flash, window did not close, no progress bar, nothing but spinning beachball.

2) Beachball spun for a LONG time with no apparent progress.

3) Simultaneously the Command+Option+Esc, Force Quit Applications window showed "Not responding" for Camino.

After multiple tries, I decided to try to import from Explorer where I have about 3,000
bookmarks. Same apparent results but just as I was about to Force Quit Explorer, all of a sudden, Camino's bookmarks window filled up.

So I decided to try Firefox again (where I have about 5,000 bookmarks) but just let it run a while even though Mac says "Not responding".  In about 3 to 4 minutes of spinning beachball, all bookmarks successfully imported.

I believe that 1.-3. above should still be reported as "minor" to "medium" level bugs because the Mac OS and Camino are not interfacing the way a Mac user would expect.

However, a simple footnote to the effect that, "Note: When importing large numbers of bookmarks, Camino bookmark window may not close, spinning beachball may occur and Mac "Force Quit" window may say Camino is "Not responding", however let Import run at least 5 minutes, as it is probably importing. 

Comment 12

[John Spooner]

ATTN:  Mike Pinkerton
From:  John Spooner (original bug reporter)
Subject: Bug "resolved"

My mistake. Import simply needed to run longer and reacted oddly but DID successfully import. Please see my amendments Comment #11).

Thanks, 

John

(In reply to comment #8)
> dan, would it be possible to pull tomorrow's (12/31) nightly build (or any date
> after that) and verify this fix?
> 

Comment 13

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

John, can you file a new bug on your comment 11 (Camino appears to be unresponsive/hung when importing huge bookmarks file) and attach your Fx bookmark file?  It seems at the very least we should be showing our own wait cursor there...

Mike, did this really land on the MOZILLA_1_8_BRANCH?  There's nothing in Bonsai for it, only for the trunk and the 1.8.0 branch.

Comment 14

[Mark Mentovai]

Removing fixed1.8.1 keyword.

Comment 15

[Mike Pinkerton (not reading bugmail)]

weird, wehn i tried it on my g5 it took like 5-10 seconds.

Comment 16

[Mike Pinkerton (not reading bugmail)]

i could swear i did. someone want to land it there?

Comment 17

[Mark Mentovai]

Checked in on 1_8 branch.

Comment 18

[Marcia Knous [:marcia]]

verified fixed on the branch using Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060110 Firefox/1.5.0.1. Adding keyword.

Comment 19

[Samuel Sidler (old account; do not CC)]

(In reply to comment #18)
> verified fixed on the branch using Mozilla/5.0 (Macintosh; U; PPC Mac OS X
> Mach-O; en-US; rv:1.8.0.1) Gecko/20060110 Firefox/1.5.0.1. Adding keyword.
> 

Marcia, please verify with a Camino build or, alternatively, don't verify at all for products you're not testing.

Comment 20

[Marcia Knous [:marcia]]

sorry, Samuel. In my haste, did not note this was a Camino bug.
(In reply to comment #19)
> (In reply to comment #18)
> > verified fixed on the branch using Mozilla/5.0 (Macintosh; U; PPC Mac OS X
> > Mach-O; en-US; rv:1.8.0.1) Gecko/20060110 Firefox/1.5.0.1. Adding keyword.
> > 
> 
> Marcia, please verify with a Camino build or, alternatively, don't verify at
> all for products you're not testing.
> 

Comment 21

[Samuel Sidler (old account; do not CC)]

Attachment: Patch for 1.7 Branch

This is an attempt at a patch for the 1.7 branch (for Camino 0.8.5). Note that this patch includes other changes which correspond to the fix for bug 309008.

Requesting review from Simon.

Comment 22

[Samuel Sidler (old account; do not CC)]

Comment on attachment 213020 [details] [diff] [review]
Patch for 1.7 Branch

Thank you Simon. :)

Requesting sr from Mike. If you approve this, please check it in as well.

Comment 23

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 213020 [details] [diff] [review]
Patch for 1.7 Branch

I tested this, and it worked (although we had a slight glitch where the + characters weren't removed by "patch" inexplicably, so just watch after checkin).

Comment 24

[Samuel Sidler (old account; do not CC)]

Attachment: Real Patch for 1.7 Branch

This version has fixed line endings.

This has r+ from smfr and smokey.

Comment 25

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 213166 [details] [diff] [review]
Real Patch for 1.7 Branch

sr=pink for 17branch

Comment 26

[Samuel Sidler (old account; do not CC)]

timeless checked this in to MOZILLA_1_7_BRANCH at 2006-02-27 22:03.