Bug 125525

Summary: [ATK] Add new layout test to check ATK roles in a central place
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch proposal cfleizach: review+, mario: commit-queue-

Description Mario Sanchez Prada 2013-12-10 11:40:37 PST
I've been playing during the hackfest with a modified version of platform/mac/accessibility/role-subrole-roledescription.html that would be very useful to have in GTK, so we have a central place where to check that the exposed roles for different kind of elements should be the right ones, as well as a place to modify slightly each time we fix issues related just to which role is exposed for objects.

I'll be attaching it soon, but here is the bug
Comment 1 Mario Sanchez Prada 2013-12-10 11:53:07 PST
Created attachment 218887 [details]
Patch proposal

Here is the patch. The idea is to get this in now and file tomorrow some more bugs fixing particular issues from those skipped things: landmarks, ARIA roles, checmenuitem...

The  patches are already ready, but I need to double chek some things first, so that's why I'm not doing it now
Comment 2 chris fleizach 2013-12-10 13:26:49 PST
Comment on attachment 218887 [details]
Patch proposal

is it possible to use the same Mac test, but with different expectations? it'd be nice to only modify one test and generate new expectations
Comment 3 Mario Sanchez Prada 2013-12-10 18:01:18 PST
(In reply to comment #2)
> (From update of attachment 218887 [details])
> is it possible to use the same Mac test, but with different expectations? it'd > be nice to only modify one test and generate new expectations

Yes, that was my first approach, but I don't think it's possible to reuse the Mac test just by providing new expectations mainly because the accessibility hierarchy is not equivalent (e.g. ATK might not expose elements exposed for Mac, and viceversa), so the list of elements that you might want to skip/check will differ in several cases.

Also, there's the additional problem of this test checking subroles and role descriptions too, which are concepts that do not exist in ATK. So, we would either have to provide a lot of FAIL expectations for ATK or we would need to heavily change the javascript to better work in both platforms, maybe by removing all those data-* attributes for the expectations and using debug() all around. 

But again, even if that might work for some cases, I think in some others it will be more messy than beneficial, as the current approach of using data-* seems quite neat to me.

So, all in all, I thought this would be the least bad solution, even though I agree it's not ideal.

What do you think?
Comment 4 chris fleizach 2013-12-11 09:26:18 PST
Comment on attachment 218887 [details]
Patch proposal

I think it might be cool if each element had a platform-supported attribute that listed = "mac, atk" or "ark" or whatever to handle the platform differences
I think we could also conditionalize the roleDescription and subRole to only be called on the mac.
Comment 5 Mario Sanchez Prada 2013-12-11 09:49:42 PST
(In reply to comment #4)
> (From update of attachment 218887 [details])
> I think it might be cool if each element had a platform-supported attribute that listed = "mac, atk" or "ark" or whatever to handle the platform differences
> I think we could also conditionalize the roleDescription and subRole to only be called on the mac.

That might be an option, although I'm afraid that's something that would take a bit more of time and I would like, if possible, to get this GTK specific test in asap (ideally during the hackfest), so I can also push the other patches that I have depending on it, which make the GTK role expose the proper roles for a few cases more (e.g. Landmarks).

So, if it sounds good to you, I think we could maybe get this patch in now (it's GTK specific after all), then try to land the other ones that I have in the pipeline (so WebKitGTK+ exposes more roles properly) and, after that, revisit this issue of merging this test and the Mac one into only one? I would of course file a bug for that, and would try to work on it already during the next week.

How does it sound?
Comment 6 chris fleizach 2013-12-11 09:58:38 PST
Comment on attachment 218887 [details]
Patch proposal

Ok
Comment 7 Mario Sanchez Prada 2013-12-11 10:25:12 PST
Comment on attachment 218887 [details]
Patch proposal

Thanks for your understanding
Comment 8 Mario Sanchez Prada 2013-12-11 10:50:05 PST
Comment on attachment 218887 [details]
Patch proposal

I will land it manually in the end
Comment 9 Mario Sanchez Prada 2013-12-11 10:55:35 PST
Committed r160447: <http://trac.webkit.org/changeset/160447>