333579 - Firefox crash with Window-Eyes and DHTML/AJAX

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Dan Kinnunen]

Env: WE 5.5b
     FF gecko 20060406
     The URL is http://www.datashelf.us/ticTacToe.html
     

Problem: If you play the game tictactoe with Window-Eyes 5.5 and then exit Firefox, Firefox will crash during the exit.   I've sent a talkbacks relating to 
this crash. While the game is playing it works great, but when you're done the problem occurs.

Steps to recreate: Start WE, start FF and load URL http://www.datashelf.us/ticTacToe.html.
 Play game a few times and exit FF, you will notice that FF will crash. before exiting properly.

Comment 1

[Dan Kinnunen]

The talkback incident ID is 17444683

Comment 2

[Aaron Leventhal]

Bill, I'm unable to get this to crash with Firefox 2 alphas. We've been checking in some crash and leak fixes, and might have already fixed it.

WFM

Comment 3

[Aaron Leventhal]

Talkback 17848815
http://talkback-public.mozilla.org/search/start.jsp?search=2&type=iid&id=17848815

Bill, are you in browse mode on or off when you are doing this?

Comment 4

[Dan Kinnunen]

Update, I was in browse mode on when I saw this problem.

Comment 5

[Aaron Leventhal]

Possibly related: bug 337167

Comment 6

[Phil Ringnalda (:philor)]

Mark, I'm betting you meant to assign this to yourself, rather than to nobody@?

Comment 7

[Aaron Leventhal]

Attachment: Two fixes for image map a11y: 1) Don't walk into maps so that areas don't show twice in a11y tree, 2) Don't refer to |this| in area constructor, Init() from outside as other accessibles do

Comment 8

[[e-mail]]

Comment on attachment 228812 [details] [diff] [review]
Two fixes for image map a11y: 1) Don't walk into maps so that areas don't show twice in a11y tree, 2) Don't refer to |this| in area constructor, Init() from outside as other accessibles do

>+    // Don't walk into maps, they take up no space.
>+    // The nsHTMLAreaAccessible's they contain are attached as
>+    // children of the appropriate nsHTMLImageAccessible.
So does this make the area frame check further on redundant?

> nsHTMLAreaAccessible::nsHTMLAreaAccessible(nsIDOMNode *aDomNode, nsIAccessible *aParent, nsIWeakReference* aShell):
> nsLinkableAccessible(aDomNode, aShell)
> { 
>-  Init(); // Make sure we're in cache
>-  mParent = aParent;
> }
It took me some time to find another Init, and it turned up in nsHTMLTableHeadAccessible's constructor. Then I found one in nsXULTreeitemAccessible. Are these both wrong too? Are these different because they are creaated on demand, rather than via GetAccessible?

Comment 9

[Aaron Leventhal]

(In reply to comment #8)
> (From update of attachment 228812 [details] [diff] [review] [edit])
> >+    // Don't walk into maps, they take up no space.
> >+    // The nsHTMLAreaAccessible's they contain are attached as
> >+    // children of the appropriate nsHTMLImageAccessible.
> So does this make the area frame check further on redundant?
Not redundant, because this method is called when a focus event goes to an <area> and will return the accessible from the cache for that.

> It took me some time to find another Init, and it turned up in
> nsHTMLTableHeadAccessible's constructor. Then I found one in
> nsXULTreeitemAccessible. Are these both wrong too? 
Thanks. Yes, these are wrong because we should never refer to |this| in a constructor. Do you need a new patch with those removed or can I just fix them on checkin?

Comment 10

[[e-mail]]

Comment on attachment 228812 [details] [diff] [review]
Two fixes for image map a11y: 1) Don't walk into maps so that areas don't show twice in a11y tree, 2) Don't refer to |this| in area constructor, Init() from outside as other accessibles do

I guess those other Init()s really belong in a separate bug.

Comment 11

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 228812 [details] [diff] [review]
Two fixes for image map a11y: 1) Don't walk into maps so that areas don't show twice in a11y tree, 2) Don't refer to |this| in area constructor, Init() from outside as other accessibles do

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and marked fixed1.8.1 when you have done so.