Bug 106922 – REGRESSION (r139444): Crashes in three accessibility tests on GTK

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Zan Dobersek]

http://trac.webkit.org/changeset/139444

The affected tests:
accessibility/aria-tables.html
accessibility/aria-hidden-with-elements.html
platform/gtk/accessibility/aria-table-hierarchy.html

The backtrace of the crashing thread:
Thread 1 (Thread 0x7f97216e0900 (LWP 11963)):
#0  0x00007f972cbf127c in WTF::HashTable<WebCore::RenderBoxModelObject const*, WTF::KeyValuePair<WebCore::RenderBoxModelObject const*, WebCore::RenderBoxModelObject*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::RenderBoxModelObject const*, WebCore::RenderBoxModelObject*> >, WTF::PtrHash<WebCore::RenderBoxModelObject const*>, WTF::HashMapValueTraits<WTF::HashTraits<WebCore::RenderBoxModelObject const*>, WTF::HashTraits<WebCore::RenderBoxModelObject*> >, WTF::HashTraits<WebCore::RenderBoxModelObject const*> >::checkKey<WTF::IdentityHashTranslator<WTF::PtrHash<WebCore::RenderBoxModelObject const*> >, WebCore::RenderBoxModelObject const*> (this=<error reading variable: Cannot access memory at address 0x7fff02651fe8>, key=<error reading variable: Cannot access memory at address 0x7fff02651fe0>) at ../../Source/WTF/wtf/HashTable.h:584
#1  0x00007f972cbef6a7 in WTF::HashTable<WebCore::RenderBoxModelObject const*, WTF::KeyValuePair<WebCore::RenderBoxModelObject const*, WebCore::RenderBoxModelObject*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::RenderBoxModelObject const*, WebCore::RenderBoxModelObject*> >, WTF::PtrHash<WebCore::RenderBoxModelObject const*>, WTF::HashMapValueTraits<WTF::HashTraits<WebCore::RenderBoxModelObject const*>, WTF::HashTraits<WebCore::RenderBoxModelObject*> >, WTF::HashTraits<WebCore::RenderBoxModelObject const*> >::lookup<WTF::IdentityHashTranslator<WTF::PtrHash<WebCore::RenderBoxModelObject const*> >, WebCore::RenderBoxModelObject const*> (this=0x2c40220, key=@0x7fff026520e8: 0x2cdf978) at ../../Source/WTF/wtf/HashTable.h:602
#2  0x00007f972cbedfaf in WTF::HashTable<WebCore::RenderBoxModelObject const*, WTF::KeyValuePair<WebCore::RenderBoxModelObject const*, WebCore::RenderBoxModelObject*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::RenderBoxModelObject const*, WebCore::RenderBoxModelObject*> >, WTF::PtrHash<WebCore::RenderBoxModelObject const*>, WTF::HashMapValueTraits<WTF::HashTraits<WebCore::RenderBoxModelObject const*>, WTF::HashTraits<WebCore::RenderBoxModelObject*> >, WTF::HashTraits<WebCore::RenderBoxModelObject const*> >::lookup (this=0x2c40220, key=@0x7fff026520e8: 0x2cdf978) at ../../Source/WTF/wtf/HashTable.h:419
#3  0x00007f972cbecb8b in WTF::HashMap<WebCore::RenderBoxModelObject const*, WebCore::RenderBoxModelObject*, WTF::PtrHash<WebCore::RenderBoxModelObject const*>, WTF::HashTraits<WebCore::RenderBoxModelObject const*>, WTF::HashTraits<WebCore::RenderBoxModelObject*> >::get (this=0x2c40220, key=@0x7fff026520e8: 0x2cdf978) at ../../Source/WTF/wtf/HashMap.h:368
#4  0x00007f972cbe9bb7 in WebCore::RenderBoxModelObject::continuation (this=0x2cdf978) at ../../Source/WebCore/rendering/RenderBoxModelObject.cpp:2683
#5  0x00007f972cb543de in WebCore::RenderBlock::inlineElementContinuation (this=0x2cdf978) at ../../Source/WebCore/rendering/RenderBlock.cpp:3113
#6  0x00007f972c170d57 in WebCore::startOfContinuations (r=0x2cdf978) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:261
#7  0x00007f972c171676 in WebCore::AccessibilityRenderObject::renderParentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:436
#8  0x00007f972c171996 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:491
#9  0x00007f972c165c2d in WebCore::AccessibilityObject::parentObjectUnignored (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.cpp:345
#10 0x00007f972c1547d3 in WebCore::AccessibilityARIAGridRow::parentTable (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:123
#11 0x00007f972c189421 in WebCore::AccessibilityTableRow::isTableRow (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:72
#12 0x00007f972c1893df in WebCore::AccessibilityTableRow::roleValue (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:64
#13 0x00007f972c1630b7 in WebCore::AccessibilityObject::isWebArea (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.h:381
#14 0x00007f972c1719d7 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:496
#15 0x00007f972c165c2d in WebCore::AccessibilityObject::parentObjectUnignored (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.cpp:345
#16 0x00007f972c1547d3 in WebCore::AccessibilityARIAGridRow::parentTable (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:123
#17 0x00007f972c189421 in WebCore::AccessibilityTableRow::isTableRow (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:72
#18 0x00007f972c1893df in WebCore::AccessibilityTableRow::roleValue (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:64
#19 0x00007f972c1630b7 in WebCore::AccessibilityObject::isWebArea (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.h:381
#20 0x00007f972c1719d7 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:496
#21 0x00007f972c165c2d in WebCore::AccessibilityObject::parentObjectUnignored (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.cpp:345
#22 0x00007f972c1547d3 in WebCore::AccessibilityARIAGridRow::parentTable (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:123
#23 0x00007f972c189421 in WebCore::AccessibilityTableRow::isTableRow (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:72
#24 0x00007f972c1893df in WebCore::AccessibilityTableRow::roleValue (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:64
#25 0x00007f972c1630b7 in WebCore::AccessibilityObject::isWebArea (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.h:381
#26 0x00007f972c1719d7 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:496
... (Note that the backtrace shows a loop through the last 6 frames)

[Joanmarie Diggs]

While r139444 is officially the crasher cause; this really seems like an infinite loop that was just waiting to happen.

  0. parentObjectUnignored() calls parentObject()
  1. parentObject() calls isWebArea()
  2. isWebArea() calls roleValue()

  In the case of a table row:

  3. roleValue() calls isTableRow()
  4. isTableRow() calls parentTable() 

  For non-aria table rows, it looks at the renderer and gets the
  renderer's table and then gets the corresponding accessible from
  AXObjectCache. BUT for aria table rows, it works it's way up the
  accessible tree:

  5. parentTable() calls parentObjectUnignored()

Proposed patch comin' up.

[Joanmarie Diggs]

Created attachment 182886 [details]
Patch

[Joanmarie Diggs]

Chris and Dominic: Mind taking a look? Thanks in advance!

[chris fleizach]

Comment on attachment 182886 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=182886&action=review

> Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:127
> +    // Calling parentObjectUnignored() can result in an infinite loop when roleValue() is examined.

would parentObjectIfExists() work here?

[Joanmarie Diggs]

(In reply to comment #4)
> (From update of attachment 182886 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182886&action=review
> 
> > Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:127
> > +    // Calling parentObjectUnignored() can result in an infinite loop when roleValue() is examined.
> 
> would parentObjectIfExists() work here?

I didn't try it, but taking a look at the method:

{
    // WebArea's parent should be the scroll view containing it.
    if (isWebArea() || isSeamlessWebArea())
        return axObjectCache()->get(m_renderer->frame()->view());

    return axObjectCache()->get(renderParentObject());

It's the isWebArea() call in parentObject() that's getting us into trouble here. So my guess is that it wouldn't work. (Right?)

[chris fleizach]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 182886 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=182886&action=review
> > 
> > > Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:127
> > > +    // Calling parentObjectUnignored() can result in an infinite loop when roleValue() is examined.
> > 
> > would parentObjectIfExists() work here?
> 
> I didn't try it, but taking a look at the method:
> 
> {
>     // WebArea's parent should be the scroll view containing it.
>     if (isWebArea() || isSeamlessWebArea())
>         return axObjectCache()->get(m_renderer->frame()->view());
> 
>     return axObjectCache()->get(renderParentObject());
> 
> It's the isWebArea() call in parentObject() that's getting us into trouble here. So my guess is that it wouldn't work. (Right?)

I hope isWebArea() isn't causing a problem. all it does is

 bool isWebArea() const { return roleValue() == WebAreaRole; }

[Joanmarie Diggs]

(In reply to comment #6)

> I hope isWebArea() isn't causing a problem. all it does is
> 
>  bool isWebArea() const { return roleValue() == WebAreaRole; }

From the opening report:

#12 0x00007f972c1893df in WebCore::AccessibilityTableRow::roleValue (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:64
#13 0x00007f972c1630b7 in WebCore::AccessibilityObject::isWebArea (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.h:381
#14 0x00007f972c1719d7 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:496
#15 0x00007f972c165c2d in WebCore::AccessibilityObject::parentObjectUnignored (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.cpp:345
#16 0x00007f972c1547d3 in WebCore::AccessibilityARIAGridRow::parentTable (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:123
#17 0x00007f972c189421 in WebCore::AccessibilityTableRow::isTableRow (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:72
#18 0x00007f972c1893df in WebCore::AccessibilityTableRow::roleValue (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:64
#19 0x00007f972c1630b7 in WebCore::AccessibilityObject::isWebArea (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.h:381
#20 0x00007f972c1719d7 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:496
#21 0x00007f972c165c2d in WebCore::AccessibilityObject::parentObjectUnignored (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.cpp:345

Having said that, I suppose my previous statement isn't quite right. isWebArea() itself is not to blame; it's what happens when we call roleValue() in a table row -- in particular an ARIA table row.

For table rows we determine the role value by first looking to the parent table. In non-ARIA tables, we get the parent table by looking to the renderer, getting its table, and turning that into an accessible. Something I assume we can count on working because non-ARIA tables are, well, tables. :)

In ARIA tables, we call parentObjectUnignored() which can result in our calling isWebArea() which results in our calling roleValue() which result in our looking to the parent table which we get by parentObjectUnignored().

[chris fleizach]

Ah I see

I don't have the code handy but it occurs to me that maybe roleValue() should be const and the role for this table row should be pre determined like other objects

(In reply to comment #7)
> (In reply to comment #6)
> 
> > I hope isWebArea() isn't causing a problem. all it does is
> > 
> >  bool isWebArea() const { return roleValue() == WebAreaRole; }
> 
> From the opening report:
> 
> #12 0x00007f972c1893df in WebCore::AccessibilityTableRow::roleValue (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:64
> #13 0x00007f972c1630b7 in WebCore::AccessibilityObject::isWebArea (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.h:381
> #14 0x00007f972c1719d7 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:496
> #15 0x00007f972c165c2d in WebCore::AccessibilityObject::parentObjectUnignored (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.cpp:345
> #16 0x00007f972c1547d3 in WebCore::AccessibilityARIAGridRow::parentTable (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:123
> #17 0x00007f972c189421 in WebCore::AccessibilityTableRow::isTableRow (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:72
> #18 0x00007f972c1893df in WebCore::AccessibilityTableRow::roleValue (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityTableRow.cpp:64
> #19 0x00007f972c1630b7 in WebCore::AccessibilityObject::isWebArea (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.h:381
> #20 0x00007f972c1719d7 in WebCore::AccessibilityRenderObject::parentObject (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityRenderObject.cpp:496
> #21 0x00007f972c165c2d in WebCore::AccessibilityObject::parentObjectUnignored (this=0x2bb4c00) at ../../Source/WebCore/accessibility/AccessibilityObject.cpp:345
> 
> Having said that, I suppose my previous statement isn't quite right. isWebArea() itself is not to blame; it's what happens when we call roleValue() in a table row -- in particular an ARIA table row.
> 
> For table rows we determine the role value by first looking to the parent table. In non-ARIA tables, we get the parent table by looking to the renderer, getting its table, and turning that into an accessible. Something I assume we can count on working because non-ARIA tables are, well, tables. :)
> 
> In ARIA tables, we call parentObjectUnignored() which can result in our calling isWebArea() which results in our calling roleValue() which result in our looking to the parent table which we get by parentObjectUnignored().

[Joanmarie Diggs]

(In reply to comment #8)
> Ah I see
> 
> I don't have the code handy but it occurs to me that maybe roleValue() should be const and the role for this table row should be pre determined like other objects

I can give that a try. But I'm wondering if the way it was done as it is currently is due to differences in hierarchy, differences in how tables get exposed on different platforms, etc.

So do you think looking to the render objects as I did in my patch is a bad idea?

[chris fleizach]

I think your proposed solution gets the job done, I'm just trying to think if we can address the issue at a more fundamental level

[Joanmarie Diggs]

Created attachment 182986 [details]
Patch

[Joanmarie Diggs]

(In reply to comment #10)
> I think your proposed solution gets the job done, I'm just trying to think if we can address the issue at a more fundamental level

But a one-line fix takes all the fun out of it. ;) ;) ;) ;)

Ok, I tried what you suggested and it works on the Gtk port. Let's see if the EWS spits up.

And thanks for the suggestion!

[Joanmarie Diggs]

Comment on attachment 182986 [details]
Patch

Chris, the EWS didn't spit up. The fix is a one-liner which does (I think) what you suggested. :)

Please review. Thanks!

[chris fleizach]

Thanks. I'm only concerned with if there are cases where an ARIA grid row is not a RowRole.

I am thinking whether the spec states that a role="grid row" should not be a row if it does not have a parent that is a table

ie)

<div>
<div role="gridrow">row</div>
</div>

should the row show up as a RowRole out of the table context? 

Part of me thinks there was something in the spec that stated this should not happen

[Joanmarie Diggs]

(In reply to comment #14)
> Thanks. I'm only concerned with if there are cases where an ARIA grid row is not a RowRole.
> 
> I am thinking whether the spec states that a role="grid row" should not be a row if it does not have a parent that is a table
> 
> ie)
> 
> <div>
> <div role="gridrow">row</div>
> </div>
> 
> should the row show up as a RowRole out of the table context? 
> 
> Part of me thinks there was something in the spec that stated this should not happen

In which case, we would need to check the parentTable, would we not? Checking the parentTable is what was being done before (though with the infinite loop). My original patch continues to check the parentTable (though avoids the infinite loop).

Is there a third option which both addresses the concern you mention here whilst still addresses the problem at a more fundamental level as you suggested before?

[chris fleizach]

maybe we could check the parent table in initialization?

[Joanmarie Diggs]

(In reply to comment #16)
> maybe we could check the parent table in initialization?

You know of any code I can look at to see this being done?

Also what happens if a web app changes the tree? Does that matter?

[chris fleizach]

(In reply to comment #17)
> (In reply to comment #16)
> > maybe we could check the parent table in initialization?
> 
> You know of any code I can look at to see this being done?
> 
> Also what happens if a web app changes the tree? Does that matter?

You might be able to just override

AccessibilityRole AccessibilityRenderObject::determineAccessibilityRole()

in AccessibilityTableRow (I bet this problem happens to regular tables too, not just ARIA tables)

and put the roleValue() logic in there. 

if the web app changes the role of the object on the fly, i think we clear the object and it is remade a new

[Joanmarie Diggs]

Created attachment 183040 [details]
Patch

[Joanmarie Diggs]

(In reply to comment #18)
> (In reply to comment #17)
> > (In reply to comment #16)
> > > maybe we could check the parent table in initialization?
> > 
> > You know of any code I can look at to see this being done?
> > 
> > Also what happens if a web app changes the tree? Does that matter?
> 
> You might be able to just override
> 
> AccessibilityRole AccessibilityRenderObject::determineAccessibilityRole()

Awesome. Thank you!! Works on Gtk. Let's see what the EWS thinks.

[chris fleizach]

(In reply to comment #20)
> (In reply to comment #18)
> > (In reply to comment #17)
> > > (In reply to comment #16)
> > > > maybe we could check the parent table in initialization?
> > > 
> > > You know of any code I can look at to see this being done?
> > > 
> > > Also what happens if a web app changes the tree? Does that matter?
> > 
> > You might be able to just override
> > 
> > AccessibilityRole AccessibilityRenderObject::determineAccessibilityRole()
> 
> Awesome. Thank you!! Works on Gtk. Let's see what the EWS thinks.

I like this version

[Build Bot]

Comment on attachment 183040 [details]
Patch

Attachment 183040 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15909633

New failing tests:
platform/mac/accessibility/search-when-element-starts-in-table.html

[Build Bot]

Comment on attachment 183040 [details]
Patch

Attachment 183040 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15926565

New failing tests:
platform/mac/accessibility/search-when-element-starts-in-table.html

[Joanmarie Diggs]

(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #18)
> > > (In reply to comment #17)
> > > > (In reply to comment #16)
> > > > > maybe we could check the parent table in initialization?
> > > > 
> > > > You know of any code I can look at to see this being done?
> > > > 
> > > > Also what happens if a web app changes the tree? Does that matter?
> > > 
> > > You might be able to just override
> > > 
> > > AccessibilityRole AccessibilityRenderObject::determineAccessibilityRole()
> > 
> > Awesome. Thank you!! Works on Gtk. Let's see what the EWS thinks.
> 
> I like this version

One of the mac tests didn't. Any idea why?

[Joanmarie Diggs]

(In reply to comment #24)
 
> > I like this version
> 
> One of the mac tests didn't. Any idea why?

Having looked at the test that failed, we examine the cell and the row, but not the table. So I'm guessing an accessible object hasn't been created for it. Which brings me to the following:

The new AccessibilityTableRow::determineAccessibilityRole() does this:

    if (!isTableRow())
        return AccessibilityRenderObject::determineAccessibilityRole();

isTableRow() is true only if parentTable() returns an AccessibilityTable.

parentTable() has this code in it:

    // Do not use getOrCreate. parentTable() can be called while the render tree is being modified.
    return axObjectCache()->get(toRenderTableRow(m_renderer)->table());

And get() ain't returning a table.

As an experiment, I ignored the comment and called getOrCreate() and the failing test now passes. Assuming the comment is not lying, time to find a clever solution....

[chris fleizach]

(In reply to comment #25)
> (In reply to comment #24)
> 
> > > I like this version
> > 
> > One of the mac tests didn't. Any idea why?
> 
> Having looked at the test that failed, we examine the cell and the row, but not the table. So I'm guessing an accessible object hasn't been created for it. Which brings me to the following:
> 
> The new AccessibilityTableRow::determineAccessibilityRole() does this:
> 
>     if (!isTableRow())
>         return AccessibilityRenderObject::determineAccessibilityRole();
> 
> isTableRow() is true only if parentTable() returns an AccessibilityTable.
> 
> parentTable() has this code in it:
> 
>     // Do not use getOrCreate. parentTable() can be called while the render tree is being modified.
>     return axObjectCache()->get(toRenderTableRow(m_renderer)->table());
> 
> And get() ain't returning a table.
> 
> As an experiment, I ignored the comment and called getOrCreate() and the failing test now passes. Assuming the comment is not lying, time to find a clever solution....

You could make a parentTable() method that gets and one that getsOrCreates(), then use the later in the determineRole field. 

The only question is whether there's a problem creating a parent object from within a child object

[Joanmarie Diggs]

> You could make a parentTable() method that gets and one that getsOrCreates(), then use the later in the determineRole field. 
> 
> The only question is whether there's a problem creating a parent object from within a child object

I may have an even better idea. It works on Gtk+, it works on my Mac and involves removing code rather than adding it. :) Patch coming soon.

Remember the original problem was that for ARIA we had an endless loop because of the call to parentObjectUnignored(). And that problem was solved thanks to your suggestion of moving that logic into determineAccessibilityRole(). Given any accessible table row, the unignored parent should be an accessible table. And that parent should not be far away element-wize. So why not use that approach for all table rows?

[Joanmarie Diggs]

Created attachment 183264 [details]
Patch

[Joanmarie Diggs]

Comment on attachment 183264 [details]
Patch

To review my own patch, I just moved this bit of code and parent should be non-null. But my paranoia wants to add a null-check just in case. So I think I'll do that -- after code review on the rest.

[chris fleizach]

Comment on attachment 183264 [details]
Patch

i think this is good, but something happened with the bots. we should try to get the bots to process this just in case

[Joanmarie Diggs]

Created attachment 183285 [details]
Patch

[Joanmarie Diggs]

(In reply to comment #30)
> (From update of attachment 183264 [details])
> i think this is good, but something happened with the bots. we should try to get the bots to process this just in case

Agreed. Martin Robinson suggested I resubmit the patch and since I wanted to add the null check, I did that as well. So if you're happy and EWS is happy, then I am happy with what I just attached.

[Joanmarie Diggs]

Comment on attachment 183285 [details]
Patch

All bots are green. The Gtk bot has non-patch related issues at the moment. Given that this bug is causing crashes (not just in tests, not just with people using ATs) and is in the current unstable release of WebKitGtk, if there are no other issues it would be great to get this resolved.

Thanks again Chris for all your help on this! :)

[WebKit Review Bot]

Comment on attachment 183285 [details]
Patch

Clearing flags on attachment: 183285

Committed r140095: <http://trac.webkit.org/changeset/140095>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Sergio Villar Senin]

Unfortunately I keep experiencing a11y related crashes quite frequently:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff510a14b in WebCore::AccessibilityTableCell::parentTable() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
(gdb) bt
#0  0x00007ffff510a14b in WebCore::AccessibilityTableCell::parentTable() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#1  0x00007ffff510a09d in WebCore::AccessibilityTableCell::isTableCell() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#2  0x00007ffff510a0dd in WebCore::AccessibilityTableCell::roleValue() const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#3  0x00007ffff5f8d106 in webkitAccessibleDetach () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#4  0x00007ffff510de3c in WebCore::AXObjectCache::remove(unsigned int) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#5  0x00007ffff510e05e in WebCore::AXObjectCache::remove(WebCore::RenderObject*) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#6  0x00007ffff58bb4ac in WebCore::RenderObject::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#7  0x00007ffff57dbd5d in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#9  0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#10 0x00007ffff58bb1fa in WebCore::RenderObject::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#11 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#12 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#13 0x00007ffff58bb1fa in WebCore::RenderObject::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#14 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#15 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#16 0x00007ffff57dbc7c in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#17 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#18 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#19 0x00007ffff57dbc7c in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#20 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#21 0x00007ffff58b1b04 in WebCore::RenderObjectChildList::destroyLeftoverChildren() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#22 0x00007ffff57dbc7c in WebCore::RenderBlock::willBeDestroyed() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#23 0x00007ffff58b9e5d in WebCore::RenderObject::destroy() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#24 0x00007ffff5346ac2 in WebCore::Node::detach() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#25 0x00007ffff52d6b6e in WebCore::ContainerNode::detach() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#26 0x00007ffff5322734 in WebCore::Element::detach() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#27 0x00007ffff53232cc in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#28 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#29 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#30 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#31 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#32 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#33 0x00007ffff5322fe5 in WebCore::Element::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#34 0x00007ffff52f72db in WebCore::Document::recalcStyle(WebCore::Node::StyleChange) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#35 0x00007ffff52f76ce in WebCore::Document::updateStyleIfNeeded() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#36 0x00007ffff52f8212 in WebCore::Document::updateLayout() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#37 0x00007ffff52fa939 in WebCore::Document::updateLayoutIgnorePendingStylesheets() () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#38 0x00007ffff572590b in WebCore::DOMWindow::scrollTo(int, int) const () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0
#39 0x00007ffff5af5848 in WebCore::jsDOMWindowPrototypeFunctionScrollTo(JSC::ExecState*) () from ~/opt/gnome3/lib64/libwebkitgtk-3.0.so.0

[Joanmarie Diggs]

Different bug.