Bug 120456 - [AX][ATK] Added support for sort and help attributes.
Summary: [AX][ATK] Added support for sort and help attributes.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-29 01:53 PDT by Krzysztof Czech
Modified: 2013-09-03 04:28 PDT (History)
8 users (show)

See Also:


Attachments
Patch (21.26 KB, patch)
2013-08-29 02:07 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Patch (21.24 KB, patch)
2013-08-29 02:24 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff
Patch (21.25 KB, patch)
2013-08-30 01:21 PDT, Krzysztof Czech
cfleizach: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Patch (20.98 KB, patch)
2013-09-03 03:14 PDT, Krzysztof Czech
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Krzysztof Czech 2013-08-29 01:53:50 PDT
Added support for aria-help and aria-sort attributes.
Sharing aria-sort.html specific mac test with other ports.
Comment 1 Radar WebKit Bug Importer 2013-08-29 01:54:11 PDT
<rdar://problem/14866030>
Comment 2 Krzysztof Czech 2013-08-29 02:07:12 PDT
Created attachment 209960 [details]
Patch
Comment 3 WebKit Commit Bot 2013-08-29 02:08:36 PDT
Attachment 209960 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/accessibility/aria-sort-expected.txt', u'LayoutTests/accessibility/aria-sort.html', u'LayoutTests/platform/efl-wk2/TestExpectations', u'LayoutTests/platform/efl-wk2/accessibility/image-link-expected.txt', u'LayoutTests/platform/efl-wk2/accessibility/image-map2-expected.txt', u'LayoutTests/platform/efl-wk2/accessibility/table-cell-spans-expected.txt', u'LayoutTests/platform/efl-wk2/accessibility/table-cells-expected.txt', u'LayoutTests/platform/gtk/accessibility/image-link-expected.txt', u'LayoutTests/platform/gtk/accessibility/image-map2-expected.txt', u'LayoutTests/platform/gtk/accessibility/table-cell-spans-expected.txt', u'LayoutTests/platform/gtk/accessibility/table-cells-expected.txt', u'LayoutTests/platform/mac/accessibility/aria-sort-expected.txt', u'LayoutTests/platform/mac/accessibility/aria-sort.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp', u'Tools/ChangeLog', u'Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp', u'Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp']" exit_code: 1
Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:55:  An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Krzysztof Czech 2013-08-29 02:24:00 PDT
Created attachment 209961 [details]
Patch
Comment 5 chris fleizach 2013-08-29 09:03:44 PDT
Comment on attachment 209961 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +              accessibility/aria-help.html

the change log says aria-help is added, but i don't see it in this patch

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:54
> +    return attributeString == "AXSortDirection" ? "aria-sort" : String();

since it seems like you'll continue adding to this method you should probably add sort direction to the other list of if statements directly above, and then leave the return "" as the final line

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:58
> +    return attributeString == "AXSortDirection" ? "aria-sort" : String();

ditto with comment from DRT
Comment 6 Krzysztof Czech 2013-08-30 01:21:01 PDT
Created attachment 210076 [details]
Patch
Comment 7 Krzysztof Czech 2013-08-30 01:23:17 PDT
> > Source/WebCore/ChangeLog:11
> > +              accessibility/aria-help.html
> 
> the change log says aria-help is added, but i don't see it in this patch
Yes right, I did not catch this.
> 
> > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:54
> > +    return attributeString == "AXSortDirection" ? "aria-sort" : String();
> 
> since it seems like you'll continue adding to this method you should probably add sort direction to the other list of if statements directly above, and then leave the return "" as the final line
Yes, it seems other attributes will be added. Sounds good.
> 
> > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:58
> > +    return attributeString == "AXSortDirection" ? "aria-sort" : String();
> 
> ditto with comment from DRT
Sounds good.
Comment 8 WebKit Commit Bot 2013-09-02 09:25:26 PDT
Comment on attachment 210076 [details]
Patch

Rejecting attachment 210076 [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', 'apply-attachment', '--no-update', '--non-interactive', 210076, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
 file Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp.rej
patching file Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp
Hunk #1 FAILED at 52.
Hunk #2 succeeded at 698 (offset 6 lines).
1 out of 2 hunks FAILED -- saving rejects to file Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp.rej

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Chris Fleizach']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/1603987
Comment 9 Krzysztof Czech 2013-09-03 03:14:25 PDT
Created attachment 210345 [details]
Patch
Comment 10 WebKit Commit Bot 2013-09-03 04:28:30 PDT
Comment on attachment 210345 [details]
Patch

Clearing flags on attachment: 210345

Committed r154976: <http://trac.webkit.org/changeset/154976>
Comment 11 WebKit Commit Bot 2013-09-03 04:28:33 PDT
All reviewed patches have been landed.  Closing bug.