Bug 145241 - AX: UI Automation cannot find AutoFill or search cancel buttons
Summary: AX: UI Automation cannot find AutoFill or search cancel buttons
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-05-20 19:24 PDT by Daniel Bates
Modified: 2015-06-26 15:29 PDT (History)
15 users (show)

See Also:


Attachments
Work-in-progress patch (1.92 KB, patch)
2015-05-20 19:27 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch and layout test (10.22 KB, patch)
2015-05-22 16:47 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (729.62 KB, application/zip)
2015-05-22 17:03 PDT, Build Bot
no flags Details
Patch and layout tests (13.22 KB, patch)
2015-06-19 19:52 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2015-05-20 19:24:10 PDT
We should consider the AutoFill button and <input type="search"> cancel button to be hit testable.
Comment 1 Radar WebKit Bug Importer 2015-05-20 19:24:43 PDT
<rdar://problem/21051411>
Comment 2 Daniel Bates 2015-05-20 19:27:49 PDT
Created attachment 253495 [details]
Work-in-progress patch

Need to add change log entry. I am open to suggestions on the approach.
Comment 3 chris fleizach 2015-05-20 22:03:46 PDT
Comment on attachment 253495 [details]
Work-in-progress patch

I think you should follow the model of image maps, where you identify its a text field and then have a method that handles sub element hit testing if necessary
Comment 4 Daniel Bates 2015-05-22 16:47:56 PDT
Created attachment 253616 [details]
Patch and layout test
Comment 5 chris fleizach 2015-05-22 16:58:40 PDT
Comment on attachment 253616 [details]
Patch and layout test

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

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2239
> +        return accessibilityTextFieldDecoration(downcast<HTMLInputElement>(*node), *hitTestResult.innerNode());

will this still return the text field if the textFieldDecoration returns null?

i would also name

accessibilityTextFieldDecoration -> accessibilityTextFieldDecorationHitTest to match the other methods

you could also make that an instance method so that you can access the axObjectCache() directly instead of getting it through inputElement.document(). 
That probably doesn't impact anything though

> LayoutTests/platform/wk2/TestExpectations:53
> +webkit.org/b/71298 accessibility/input-search-cancel-button.html [ Failure ]

do we know why this fails on wk2? that seems like something we shouldn't ignore.

thanks
Comment 6 Build Bot 2015-05-22 17:03:22 PDT
Comment on attachment 253616 [details]
Patch and layout test

Attachment 253616 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6679441056989184

New failing tests:
platform/mac/accessibility/html-slider-indicator.html
Comment 7 Build Bot 2015-05-22 17:03:27 PDT
Created attachment 253619 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 8 Daniel Bates 2015-06-01 15:39:57 PDT
Comment on attachment 253616 [details]
Patch and layout test

Clearing review flag. Will update patch and address remarks shortly.
Comment 9 Daniel Bates 2015-06-19 19:37:46 PDT
(In reply to comment #5)
> > Source/WebCore/accessibility/AccessibilityRenderObject.cpp:2239
> > +        return accessibilityTextFieldDecoration(downcast<HTMLInputElement>(*node), *hitTestResult.innerNode());
> 
> will this still return the text field if the textFieldDecoration returns
> null?
> 

No, it will not return the text field. Will fix.

> i would also name
> 
> accessibilityTextFieldDecoration -> accessibilityTextFieldDecorationHitTest
> to match the other methods
> 

Will rename.

> you could also make that an instance method so that you can access the
> axObjectCache() directly instead of getting it through
> inputElement.document(). 
> That probably doesn't impact anything though
> 

Will make it an instance method and access axObjectCache() directly.

> > LayoutTests/platform/wk2/TestExpectations:53
> > +webkit.org/b/71298 accessibility/input-search-cancel-button.html [ Failure ]
> 
> do we know why this fails on wk2? that seems like something we shouldn't
> ignore.
> 

It fails because of bug #71298. See bug 71298, comment 53 for more details.
Comment 10 Daniel Bates 2015-06-19 19:52:14 PDT
Created attachment 255271 [details]
Patch and layout tests
Comment 11 Daniel Bates 2015-06-22 09:17:48 PDT
Comment on attachment 255271 [details]
Patch and layout tests

Clearing flags on attachment: 255271

Committed r185828: <http://trac.webkit.org/changeset/185828>
Comment 12 Daniel Bates 2015-06-22 09:17:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Alex Christensen 2015-06-23 10:26:28 PDT
This caused 3 tests to fail on Windows:
accessibility/hit-test-input-auto-fill-button.html
accessibility/hit-test-input-search-cancel-button.html
accessibility/input-search-cancel-button.html
Comment 14 Daniel Bates 2015-06-23 11:45:09 PDT
(In reply to comment #13)
> This caused 3 tests to fail on Windows:
> accessibility/hit-test-input-auto-fill-button.html
> accessibility/hit-test-input-search-cancel-button.html
> accessibility/input-search-cancel-button.html

Filed bug #146243 to look into these failures. For now, I'll skip the tests.
Comment 15 Daniel Bates 2015-06-23 11:56:57 PDT
(In reply to comment #14)
> (In reply to comment #13)
> > This caused 3 tests to fail on Windows:
> > accessibility/hit-test-input-auto-fill-button.html
> > accessibility/hit-test-input-search-cancel-button.html
> > accessibility/input-search-cancel-button.html
> 
> Filed bug #146243 to look into these failures. For now, I'll skip the tests.

Actually, I chose to mark the tests as failing and committed this in <http://trac.webkit.org/changeset/185881>.
Comment 16 Daniel Bates 2015-06-26 15:29:48 PDT
Rolled out <http://trac.webkit.org/changeset/185881> and <http://trac.webkit.org/changeset/185828> in <http://trac.webkit.org/changeset/186011> because it caused a regression.