Bug 58040 - [GTK] Unskip accessibility/input-slider.html and accessibility/media-element.html
Summary: [GTK] Unskip accessibility/input-slider.html and accessibility/media-element....
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: 58039
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-07 06:55 PDT by Mario Sanchez Prada
Modified: 2011-04-11 12:44 PDT (History)
0 users

See Also:


Attachments
Patch proposal + unskipped layout tests (12.77 KB, patch)
2011-04-07 07:05 PDT, Mario Sanchez Prada
cfleizach: review+
Details | Formatted Diff | Diff
Patch proposal + unskipped layout tests (14.06 KB, patch)
2011-04-08 09:42 PDT, Mario Sanchez Prada
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2011-04-07 06:55:11 PDT
Currently, two issues are preventing the GTK port from unskipping these two tests:

  A. GTK's DRT doesn't provide implementations for the AccessibilityUIElement's increment() and decrement() methods.
  B. AccessibilityObject's of role 'SliderThumbRole' (value indicators for sliders) shouldn't be exposed to ATK-based Assistive technologies.

About (A), I've just filed bug 58039, provided a patch for that and marked it as a dependency of this one.

As for (B), which is what this bug is actually for, we'll need to make some changes in how objects of role 'SliderThumbRole' are added to sliders, and to specify that we want to ignore those in the GTK port. Besides, we'll need slightly different checks for GTK in the case of the input-slider.html test case, so we'll probably need to move it from accessibility/ to platform/mac/accessibility, and handle our own copy inside platform/gtk/accessibility.

Attaching a patch in some minutes then...
Comment 1 Mario Sanchez Prada 2011-04-07 07:05:33 PDT
Created attachment 88627 [details]
Patch proposal + unskipped layout tests

Attaching patch proposal.
Comment 2 chris fleizach 2011-04-08 08:27:03 PDT
Comment on attachment 88627 [details]
Patch proposal + unskipped layout tests

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

r=me. I suspect you'd rather call accessibilityIsIgnored() than accessibilityPlatformIncludesObject() which i think should really only be called by accessibilityIsIgnored() itself

> Source/WebCore/accessibility/AccessibilitySlider.cpp:99
> +    if (thumb->accessibilityPlatformIncludesObject() == IgnoreObject)

can you just ask accessibilityIsIgnored() here.
Comment 3 Mario Sanchez Prada 2011-04-08 09:31:13 PDT
(In reply to comment #2)
> (From update of attachment 88627 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88627&action=review
> 
> r=me. I suspect you'd rather call accessibilityIsIgnored() than accessibilityPlatformIncludesObject() which i think should really only be called by accessibilityIsIgnored() itself
> 
> > Source/WebCore/accessibility/AccessibilitySlider.cpp:99
> > +    if (thumb->accessibilityPlatformIncludesObject() == IgnoreObject)
> 
> can you just ask accessibilityIsIgnored() here.

Problem is that currently accessibilityIsIgnored() is a private method for AXSliderThumb which always return 'false' and, even if I could change that implementation to call accessibilityIsIgnoredBase(), AXSliderThumb is a subclass of AXObject (not AXRenderObject), where that function is not defined either (accessibilityIsIgnoredBase is defined for AXRenderObject)... 

So that's why I directly called accessibilityPlatformIncludesObject() instead, following the lead of AccessibilityMenuList::addChildren().

However, I think I could slightly change my patch to:

  1. Implement accessibilityIsIgnored() in AXSliderThumb and make it public (so I can call it from AXSlider:addChildren())
  2. Make AXSliderThumb::accessibilityIsIgnored() call accessibilityPlatformIncludesObject() from there, which is defined for AXObject.

I'll make that change and propose a new patch then...
Comment 4 Mario Sanchez Prada 2011-04-08 09:42:35 PDT
Created attachment 88830 [details]
Patch proposal + unskipped layout tests

Attached new patch with the changes commented before. Asking for review again, and sorry for the hassle...
Comment 5 chris fleizach 2011-04-11 11:25:38 PDT
Comment on attachment 88830 [details]
Patch proposal + unskipped layout tests

thanks. r=me
Comment 6 Mario Sanchez Prada 2011-04-11 12:44:32 PDT
Committed r83479: <http://trac.webkit.org/changeset/83479>