Bug 38465

Summary: Misleading variable name in a11y test
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Proposed patch
darin: review+, commit-queue: commit-queue-
Proposed patch none

Mario Sanchez Prada
Reported 2010-05-03 07:58:45 PDT
In LayoutTests/accessibility/aria-controls-with-tabs.html, the variable 'body' is misleading, because it actually contain the root accessible element, not the accessible object associated to the <body> tag: var body = accessibilityController.rootElement; If you compare this with other a11y tests using the rootElement method, it's done in a different, but more understandable, way. For instance, in LayoutTests/platform/mac/accessibility/element-focus.html: var root = accessibilityController.rootElement; var body = root.childAtIndex(0); In the case of aria-controls-with-tabs.html, the test is still working fine because at the end 'body' is not being used as if it were the a11y object for body, but its parent: var body = accessibilityController.rootElement; var tabList = body.childAtIndex(0).childAtIndex(0); Hence, even though it's working fine because of the thing pointed out above, IMHO it would be better if the test was written more like LayoutTests/platform/mac/accessibility/element-focus.html
Attachments
Proposed patch (1.90 KB, patch)
2010-05-03 08:03 PDT, Mario Sanchez Prada 		darin: review+
commit-queue: commit-queue-
DetailsFormatted DiffDiff
Proposed patch (2.07 KB, patch)
2010-05-18 00:48 PDT, Mario Sanchez Prada 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mario Sanchez Prada
Comment 1 2010-05-03 08:03:46 PDT
Created attachment 54928 [details] Proposed patch Attaching a patch proposal for this
WebKit Commit Bot
Comment 2 2010-05-15 10:23:49 PDT
Comment on attachment 54928 [details] Proposed patch Rejecting patch 54928 from commit-queue. Unexpected failure when landing patch! Please file a bug against webkit-patch. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 54928, '--test', '--parent-command=commit-queue', '--no-update']" exit_code: 1 Logging in as eseidel@chromium.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=54928&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=38465&ctype=xml Processing 1 patch from 1 bug. Cleaning working directory Processing patch 54928 from bug 38465. ERROR: /Users/eseidel/Projects/CommitQueue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Mario Sanchez Prada
Comment 3 2010-05-18 00:48:52 PDT
Created attachment 56331 [details] Proposed patch (In reply to comment #2) > (From update of attachment 54928 [details]) > Rejecting patch 54928 from commit-queue. Seems I uploaded the wrong patch, lacking the "Reviewed by NOBODY" line, sorry. Now uploading the right one.
WebKit Commit Bot
Comment 4 2010-05-19 22:02:42 PDT
Comment on attachment 56331 [details] Proposed patch Clearing flags on attachment: 56331 Committed r59818: <http://trac.webkit.org/changeset/59818>
WebKit Commit Bot
Comment 5 2010-05-19 22:02:47 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.