RESOLVED FIXED 125525
[ATK] Add new layout test to check ATK roles in a central place
https://bugs.webkit.org/show_bug.cgi?id=125525
Summary [ATK] Add new layout test to check ATK roles in a central place
Mario Sanchez Prada
Reported 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
Attachments
Patch proposal (28.77 KB, patch)
2013-12-10 11:53 PST, Mario Sanchez Prada
cfleizach: review+
mario: commit-queue-
Mario Sanchez Prada
Comment 1 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
chris fleizach
Comment 2 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
Mario Sanchez Prada
Comment 3 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?
chris fleizach
Comment 4 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.
Mario Sanchez Prada
Comment 5 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?
chris fleizach
Comment 6 2013-12-11 09:58:38 PST
Comment on attachment 218887 [details] Patch proposal Ok
Mario Sanchez Prada
Comment 7 2013-12-11 10:25:12 PST
Comment on attachment 218887 [details] Patch proposal Thanks for your understanding
Mario Sanchez Prada
Comment 8 2013-12-11 10:50:05 PST
Comment on attachment 218887 [details] Patch proposal I will land it manually in the end
Mario Sanchez Prada
Comment 9 2013-12-11 10:55:35 PST
Note You need to log in before you can comment on or make changes to this bug.