Bug 31035 - [GTK] some accessibility tests hitting assertion in debug builds
Summary: [GTK] some accessibility tests hitting assertion in debug builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2009-11-02 17:10 PST by Gustavo Noronha (kov)
Modified: 2009-11-04 01:31 PST (History)
4 users (show)

See Also:


Attachments
Replace assertions with sanity checks (2.30 KB, patch)
2009-11-02 21:01 PST, Joanmarie Diggs
jmalonzo: review-
Details | Formatted Diff | Diff
Get the correct Gtk+ object before attempting to turn it into an AtkObject (2.19 KB, patch)
2009-11-02 23:32 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Replace assertions with sanity checks - take 2 (2.41 KB, patch)
2009-11-03 15:11 PST, Joanmarie Diggs
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gustavo Noronha (kov) 2009-11-02 17:10:36 PST
The following tests are hitting an assertion, which is being logged to stderr, on debug builds:

accessibility/aria-controls-with-tabs.html
accessibility/nochildren-elements.html
accessibility/non-data-table-cell-title-ui-element.html
accessibility/table-notbody.html

Here's the output:

** (DumpRenderTree:26309): CRITICAL **: AtkObject* webkit_accessible_ref_child(AtkObject*, gint): assertion `static_cast<size_t>(index) < coreObject->children().size()' failed

** (DumpRenderTree:26309): CRITICAL **: atk_object_ref_accessible_child: assertion `ATK_IS_OBJECT (accessible)' failed

** (DumpRenderTree:26309): CRITICAL **: atk_object_ref_accessible_child: assertion `ATK_IS_OBJECT (accessible)' failed
Comment 1 Joanmarie Diggs 2009-11-02 21:01:31 PST
Created attachment 42359 [details]
Replace assertions with sanity checks

===================
table-notbody.html
===================

The body elements consist of a single table which:

* Has no cells
* Has style "visibility: hidden;"

As a result it's ignored as an accessible object and the web view is childless. Then along comes the layout test:

  [...]
  body.focus();
  var table = accessibilityController.focusedElement.childAtIndex(0);
  result.innerText += "Test passes if there is no crash\n\n";
  [...]

Asking for a non-existent child triggers the assertion.

=========================
nochildren-elements.html
=========================

Similar deal.

[...]
<!-- This test makes sure that these types of elements DO NOT have children.
     The test will pass if attributesOfChildren() returns nil -->
[...]

~~~~~~~~~~~~~~~

I suspect that is also what's taking place with accessibility/non-data-table-cell-title-ui-element.html. I've not fully debugged it to prove that beyond a shadow of a doubt, but the attached patch seems to solve it along with the above two tests.

This is just my opinion, but I don't think webkit_accessible_ref_child is the place for assertions like this because anyone

  * these tests
  * badly-behaved ATs
  * me typing acc.getChildAtIndex(10000000) in Accerciser's iPython console

can easily attempt to ref a child at a bogus index. As such, I think that in this particular case, it makes more sense to do some sanity checking.

As for accessibility/aria-controls-with-tabs.html: Something else is going on there. My two main, totally-unproven theories right now are the hierarchy:

  * Root ain't what the test thinks it is (perhaps its the object of ATK_ROLE_DOCUMENT_FRAME), or
  * We've got "ignored" objects sneaking in

I'll dig some more.
Comment 2 Joanmarie Diggs 2009-11-02 23:32:11 PST
Created attachment 42363 [details]
Get the correct Gtk+ object before attempting to turn it into an AtkObject

Funny thing about totally-unproven theories: There's a high probability of you're being wrong. :-) I was wrong: Test is fine, hierarchy (at least in this particular instance) is fine. Getting the rootElement, on the other hand....

This solves it for me.
Comment 3 Xan Lopez 2009-11-02 23:41:41 PST
Comment on attachment 42363 [details]
Get the correct Gtk+ object before attempting to turn it into an AtkObject

Yeah, seems the other ports do pretty much the same, so r+!
Comment 4 WebKit Commit Bot 2009-11-02 23:51:13 PST
Comment on attachment 42363 [details]
Get the correct Gtk+ object before attempting to turn it into an AtkObject

Clearing flags on attachment: 42363

Committed r50445: <http://trac.webkit.org/changeset/50445>
Comment 5 WebKit Commit Bot 2009-11-02 23:51:18 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Jan Alonzo 2009-11-03 01:35:06 PST
Still one more patch for review.
Comment 7 Jan Alonzo 2009-11-03 02:15:07 PST
Comment on attachment 42359 [details]
Replace assertions with sanity checks

> +    AccessibilityObject::AccessibilityChildrenVector children = coreObject->children();
> +    if (index < 0 || index >= children.size())
> +        return 0;
>  
>      AccessibilityObject* coreChild = coreObject->children().at(index).get();

It would be nice if we can just reuse children here instead of calling to coreObject again.

r- because it needs one more round.
Comment 8 Joanmarie Diggs 2009-11-03 15:11:15 PST
Created attachment 42426 [details]
Replace assertions with sanity checks - take 2

(In reply to comment #7)
> (From update of attachment 42359 [details])
> > +    AccessibilityObject::AccessibilityChildrenVector children = coreObject->children();
> > +    if (index < 0 || index >= children.size())
> > +        return 0;
> >  
> >      AccessibilityObject* coreChild = coreObject->children().at(index).get();
> 
> It would be nice if we can just reuse children here instead of calling to
> coreObject again.

Yup. Done. Thanks for the review!
Comment 9 Jan Alonzo 2009-11-04 00:50:11 PST
Comment on attachment 42426 [details]
Replace assertions with sanity checks - take 2

Thanks!
Comment 10 WebKit Commit Bot 2009-11-04 01:31:19 PST
Comment on attachment 42426 [details]
Replace assertions with sanity checks - take 2

Clearing flags on attachment: 42426

Committed r50506: <http://trac.webkit.org/changeset/50506>
Comment 11 WebKit Commit Bot 2009-11-04 01:31:25 PST
All reviewed patches have been landed.  Closing bug.