crash in mozilla::dom::Element::FindAttrValueIn(int, nsIAtom*, nsIAtom* const* const*, nsCaseTreatment)

RESOLVED FIXED in Firefox 43

Status

()

Core
Disability Access APIs
--
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: MarcoZ, Assigned: surkov)

Tracking

(Blocks: 1 bug, {crash})

39 Branch
mozilla43
x86
Windows 7
crash
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-c3a8e29c-af52-4c4e-b0e1-ec7e22150812.
=============================================================

This was sent to me by Birkir Gunnarsson, an accessibility friend from Deque Systems. He's experiencing this with JAWS 16, Firefox 39 on Windows 7. Alex, any ideas what could be happening here? a11y is in the stack at position 2 or 3 and down from there.
Flags: needinfo?(surkov.alexander)
(Assignee)

Comment 1

2 years ago
Created attachment 8646994 [details] [diff] [review]
patch
Assignee: nobody → surkov.alexander
Flags: needinfo?(surkov.alexander)
Attachment #8646994 - Flags: review?(mzehe)
(In reply to alexander :surkov from comment #1)
> Created attachment 8646994 [details] [diff] [review]
> patch

I guess that works around the crash, but the crash doesn't seem to make a lot of sense, and I'd expect you can still trigger it by calling Role() from other places.
(Assignee)

Comment 3

2 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #2)

> I guess that works around the crash, but the crash doesn't seem to make a
> lot of sense, and I'd expect you can still trigger it by calling Role() from
> other places.

right, there's a chance of crash. there's bunch of ways to fix that, I would prefer the one where GetCellAt() won't traverse the kids all over.
(In reply to alexander :surkov from comment #3)
> (In reply to Trevor Saunders (:tbsaunde) from comment #2)
> 
> > I guess that works around the crash, but the crash doesn't seem to make a
> > lot of sense, and I'd expect you can still trigger it by calling Role() from
> > other places.
> 
> right, there's a chance of crash. there's bunch of ways to fix that, I would
> prefer the one where GetCellAt() won't traverse the kids all over.

I have no idea what you are trying to say.
What is a better solution here?
(Assignee)

Comment 6

2 years ago
the proposed solution I believe is good, it makes stuff faster and fixes the crash. As a follow up I would make GetCellAt() to operate on cells array, since it makes Role computation much faster (this would also fixed the crash). As another option, we probably shouldn't create HTMLTableHeaderCellAccessible for ARIA grids, we need a test case for that though.
(In reply to alexander :surkov from comment #6)
> the proposed solution I believe is good, it makes stuff faster and fixes the

Ok, I miseed that the calls where looping, now this patch makes more sense.

> crash. As a follow up I would make GetCellAt() to operate on cells array,

what do you mean cells array?

> since it makes Role computation much faster (this would also fixed the
> crash). As another option, we probably shouldn't create
> HTMLTableHeaderCellAccessible for ARIA grids, we need a test case for that
> though.

it also seems kind of wierd that getting the cell next to the current one returns the same one or different ones in a loop, but maybe there's a reasonable case where that happens.
(Assignee)

Comment 8

2 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #7)

> > crash. As a follow up I would make GetCellAt() to operate on cells array,
> 
> what do you mean cells array?

either cache cells for each row like we do embedded characters in hypertext or make sure on accessible tree creation that a row doesn't contain other accessibles than cells. I think I like 2nd more.


> > since it makes Role computation much faster (this would also fixed the
> > crash). As another option, we probably shouldn't create
> > HTMLTableHeaderCellAccessible for ARIA grids, we need a test case for that
> > though.
> 
> it also seems kind of wierd that getting the cell next to the current one
> returns the same one or different ones in a loop, 

I think it happens because GetCellAt() runs through all kids, so when you ask GetCellAt(1) for a cell at 0 index then you will iterate through the zero index cell.
Comment on attachment 8646994 [details] [diff] [review]
patch

r=me.
Attachment #8646994 - Flags: review?(mzehe) → review+

Comment 10

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/08caf7dd0502
https://hg.mozilla.org/mozilla-central/rev/08caf7dd0502
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.