632144 - crash [@ GeckoUtils::GatherTextUnder(nsIDOMNode*, nsString&)]

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

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

This bug was filed from the Socorro interface and is 
report bp-575adde0-c6ce-49e4-9229-ddd612110204 .
============================================================= 

I spotted this crash in crash-stats; in the line where GeckoUtils crashes, aNode is apparently null.  Stuart also says that GetEnclosingLinkElementAndHref isn't being checked at the call site.

Comment 1

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

Attachment: Fix?

Stuart, is this the fix you had in mind?

Comment 2

[Stuart Morgan]

Comment on attachment 510765 [details] [diff] [review]
Fix?

Well, GatherTextUnder should null-check the node and return an empty string.

This case is more subtle; we should be checking here as you've done, but I'm not sure returning and thus silently failing to add a bookmarks is the right handling (vs. continuing with empty or dummy text).

Comment 3

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

Attachment: Fix? v2

I just missed you earlier; I was flailing around in the C++, so I have no confidence that that part is correct.

The BWC hunk no longer does an early return (which prevents yet-another-smokey-regression!), although I'm not positive my current solution is correct, either. It manually emulates the current automatic behavior of using the link's URL as the bookmark title when we have no(?) linkText (you can see this on the big green Download button on the home page; I'll fix that to have an alt attribute later).

We do the automagic here: http://mxr.mozilla.org/camino/source/camino/src/bookmarks/AddBookmarkDialogController.mm#57

Comment 4

[Stuart Morgan]

Comment on attachment 511300 [details] [diff] [review]
Fix? v2

sr=smorgan

I wish we knew how this happens, but not crashing is good.

Comment 5

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

Agreed on both counts.

http://hg.mozilla.org/camino/rev/f6df2b34e027 and CAMINO_2_0_BRANCH.

Comment 6

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

(In reply to comment #3)
> using the link's URL as the bookmark title when we have no(?) linkText (you can
> see this on the big green Download button on the home page; I'll fix that to
> have an alt attribute later).

And fixed that on the website, too.

Comment 7

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

2.0.7 ended up being an emergency release.