379872 - Crash [@ nsHTMLSelectOptionAccessible::GetName] when moving option around

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, which crashes Mozilla after 500ms or so after loading.
The testcase uses enhanced privileges, so you need to download it to your computer to get the crash.

Talkback ID: TB31857111M
nsHTMLSelectOptionAccessible::GetName  [mozilla/accessible/src/html/nshtmlselectaccessible.cpp, line 507]

Comment 1

[Aaron Leventhal]

Another a11y crash in <select> is bug 378612. Probably unrelated though.

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase

Aaron changed something in bug 268935 that makes the original testcase not work
anymore with current trunk builds.
This is the updated testcase, using nsIAccessibleRetrieval.

Comment 3

[Mats Palmgren (inactive)]

Simple null-pointer crash, testcase isn't sensitive and requires priviligies
to trigger, so making bug public.

Comment 4

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

The DOM node has been removed from the document, and the corresponding a11y
node has been Shutdown, thus 'mDOMNode' is null.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/accessible/src/html/nsHTMLSelectAccessible.cpp&rev=1.81&root=/cvsroot&mark=497-499#490

I suggest we the error code NS_ERROR_DOM_NOT_FOUND_ERR for this type of
error.  Then the JS exception look like:

Exception... "Node was not found"  code: "8" nsresult: "0x80530008 (NS_ERROR_DOM_NOT_FOUND_ERR)" ...

which I think is more informative than the generic NS_ERROR_FAILURE.

Other than that, I also made it return the empty string and NS_OK if
there is no label or first child text node.  I don't see why
GetName() on <option></option> should cause an exception.

The last hunk just kills a compiler warning (gcc 4.1.2 on Linux).

I still crash on the testcase after a few Reloads, but I think it's
an unrelated crash, I'll file it separately if not done already.

Comment 5

[Aaron Leventhal]

Comment on attachment 265034 [details] [diff] [review]
Patch rev. 1

You could just use NS_ERROR_FAILURE I think -- do we really need the specific DOM error? That means we'll need to change a lot. It's pretty normal for nodes to get shut down and the accessible objects to hang around until their owner releases them.

Comment 6

[Mats Palmgren (inactive)]

> You could just use NS_ERROR_FAILURE I think ...

Ok, checked in with that change.  I do think that more diversified error
codes would increase the quality of the API though, so I filed bug 381005.

Checked in to trunk at 2007-05-17 01:12 PDT.

-> FIXED

Comment 7

[Martijn Wargers (dead)]

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070517 Minefield/3.0a5pre
I don't crash with this stacktrace anymore. Instead, I crash with the stacktrace that has been filed as bug 381010.

Comment 8

[Aaron Leventhal]

The testcase doesn't crash when the fix from bug 381010 is used.

Comment 9

[Aaron Leventhal]

I should have said, when the fix from bug 387783 is used.

Comment 10

[Aaron Leventhal]

Ugh, I mean bug 377783! Time for some coffee.