Bug 58040

Summary: [GTK] Unskip accessibility/input-slider.html and accessibility/media-element.html
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal Keywords: Gtk
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 58039    
Bug Blocks:    
Attachments:
Description Flags
Patch proposal + unskipped layout tests
cfleizach: review+
Patch proposal + unskipped layout tests cfleizach: review+

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>