Bug 121675 - [ATK] Do not expose aria-help in ATK based platforms
Summary: [ATK] Do not expose aria-help in ATK based platforms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
: 106337 (view as bug list)
Depends on:
Blocks: 121684
  Show dependency treegraph
 
Reported: 2013-09-20 06:11 PDT by Mario Sanchez Prada
Modified: 2013-09-20 17:06 PDT (History)
12 users (show)

See Also:


Attachments
Patch proposal (26.25 KB, patch)
2013-09-20 06:29 PDT, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff
Patch proposal (22.77 KB, patch)
2013-09-20 08:23 PDT, Mario Sanchez Prada
cfleizach: review+
commit-queue: commit-queue-
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 2013-09-20 06:11:57 PDT
Some tests currently present in LayoutTests/accessibility (e.g aria-help, aria-describedby-on-input) need helpText() to be implemented in DRT and WKTR, which in the mac platform relies in the AXHelp accessibility attribute, normally exposing the value that AccessibilityObject::helpText() provides, which might be associated to the aria-help attribute.

However, it turns out (now I finally see it) that both the helpText concept and the aria-help attribute are Mac specific things that do not have a clear match in the ATK world, so it's pointless both to expose those 'aria-help' attributes and implement the corresponding helpText() function in DRT and WKTR.

Furthermore, the way that such an 'aria-help' attribute is now being exposed in the ATK wrapper (through an object attribute) is wrong too because of two reasons:

1. There's not such a mapping to be expected for ATs as Orca, nor documented in [1], so it's useless other than for passing the tests. But those tests relying on helpText only make sense in the Mac, so it's pointless.
2. We should not call functions that might modify the accessibility tree in webkitAccessibleGetAttributes(), such as helpText() -see bug 121558 for more info-, but check and expose information that we already have at hand, such as the value of certain attributes, the toolkit name or the ARIA state of the object. Calling helpText() here will cause to call textUnderElement(), making it slower than what it should and potentially dangerous, since textUnderElement() might modify the accessibility tree.

So, my proposal now is:

1. Get rid of the exposure of "aria-help" as an object attribute in WebKitAccesibilityWrapperAtk
2. Remove the whole support for helpText() from DRT (based in DumpRenderTreeSupportGTK) and from WKTR (based on the "aria-help" object attribute)
3. Move currently passing tests such as aria-help or aria-described-on-input to platform/mac/accessibility.

I hope to provide a patch soon.

[1] http://www.w3.org/TR/wai-aria-implementation/#mapping_state-property
Comment 1 Mario Sanchez Prada 2013-09-20 06:12:53 PDT
*** Bug 106337 has been marked as a duplicate of this bug. ***
Comment 2 Radar WebKit Bug Importer 2013-09-20 06:13:03 PDT
<rdar://problem/15039901>
Comment 3 Mario Sanchez Prada 2013-09-20 06:29:37 PDT
Created attachment 212156 [details]
Patch proposal

Proposed patch to revert this to a sane state. Please review
Comment 4 Mario Sanchez Prada 2013-09-20 08:23:00 PDT
Created attachment 212170 [details]
Patch proposal

Now proposing a patch that really applies against trunk.

Also, I removed the move of aria-describedby-on-input to Mac because, after talking to Joanie on IRC, I realized that perhaps there's a chance for us to implement helpText() in DRT and WKTR in the future, based in  ATK_RELATION_DESCRIBED_BY, which is something not implemented yet (and what I have already filed bug 121684 for)
Comment 5 Krzysztof Czech 2013-09-20 09:00:03 PDT
LGTM.
Comment 6 Mario Sanchez Prada 2013-09-20 09:44:49 PDT
Comment on attachment 212170 [details]
Patch proposal

Thanks!
Comment 7 WebKit Commit Bot 2013-09-20 09:48:24 PDT
Comment on attachment 212170 [details]
Patch proposal

Rejecting attachment 212170 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 212170, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebKit/efl/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/1924119
Comment 8 Mario Sanchez Prada 2013-09-20 14:45:36 PDT
(In reply to comment #7)
> (From update of attachment 212170 [details])
> Rejecting attachment 212170 [details] from commit-queue.
> 
> Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 212170, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
> 
> /Volumes/Data/EWS/WebKit/Source/WebKit/efl/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
> 
> Full output: http://webkit-queues.appspot.com/results/1924119

It seems I forgot the Reviewed by NOBODY line in the WebKit/efl/ChangeLog. I'll add it and land it manually then...
Comment 9 Mario Sanchez Prada 2013-09-20 14:55:11 PDT
Committed r156203: <http://trac.webkit.org/changeset/156203>
Comment 10 Alexey Proskuryakov 2013-09-20 17:06:43 PDT
Fixed the moved test in r156213.