Bug 38465 - Misleading variable name in a11y test
Summary: Misleading variable name in a11y test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-03 07:58 PDT by Mario Sanchez Prada
Modified: 2010-05-19 22:02 PDT (History)
1 user (show)

See Also:


Attachments
Proposed patch (1.90 KB, patch)
2010-05-03 08:03 PDT, Mario Sanchez Prada
darin: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Proposed patch (2.07 KB, patch)
2010-05-18 00:48 PDT, Mario Sanchez Prada
no flags 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 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
Comment 1 Mario Sanchez Prada 2010-05-03 08:03:46 PDT
Created attachment 54928 [details]
Proposed patch

Attaching a patch proposal for this
Comment 2 WebKit Commit Bot 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).
Comment 3 Mario Sanchez Prada 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.
Comment 4 WebKit Commit Bot 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>
Comment 5 WebKit Commit Bot 2010-05-19 22:02:47 PDT
All reviewed patches have been landed.  Closing bug.