Bug 131402 - AX: Initial text selection point should respect element focus.
Summary: AX: Initial text selection point should respect element focus.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.9
: P2 Normal
Assignee: Samuel White
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-04-08 15:47 PDT by Samuel White
Modified: 2014-04-09 14:15 PDT (History)
9 users (show)

See Also:


Attachments
Patch for EWS. (10.86 KB, patch)
2014-04-08 15:51 PDT, Samuel White
no flags Details | Formatted Diff | Diff
Patch. (12.83 KB, patch)
2014-04-08 16:08 PDT, Samuel White
cfleizach: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel White 2014-04-08 15:47:54 PDT
When AX is running and we have no selection, pressing an arrow key initializes selection to the beginning of the document. Initial selection should respect and key off of the focused element if one is present. This would make it possible for a user to navigate halfway down a page using tab, and then hit an arrow key to start reading text from that point.
Comment 1 Radar WebKit Bug Importer 2014-04-08 15:48:32 PDT
<rdar://problem/16558855>
Comment 2 Samuel White 2014-04-08 15:51:07 PDT
Created attachment 228904 [details]
Patch for EWS.

No review needed. Just running through EWS.
Comment 3 Samuel White 2014-04-08 16:08:25 PDT
Created attachment 228906 [details]
Patch.

Might as well update logs. :)
Comment 4 chris fleizach 2014-04-09 11:41:13 PDT
Comment on attachment 228906 [details]
Patch.

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

> LayoutTests/platform/mac/accessibility/selection-initial.html:88
> +        //

This should just be one line I think

> LayoutTests/platform/mac/accessibility/selection-initial.html:131
> +        //

ditto

> Source/WebCore/page/EventHandler.cpp:3049
> +static void setKeyboardSelection(Frame& frame, SelectionDirection direction)

should this be setInitialKeyboardSelection

> Source/WebCore/page/EventHandler.cpp:3052
> +

you can remove this empty line

> Source/WebCore/ChangeLog:10
> +        Added function to set initial selection more gracefully. Specifically, when accessibility has the need to set initial selection,

move this explanation above the line about "No New Tests'

This should say something like
"Support the case where we want to set the initial selection and there's already a focused element."
The current wording is a bit backward in terms of importance
Comment 5 Samuel White 2014-04-09 14:14:47 PDT
(In reply to comment #4)
> (From update of attachment 228906 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228906&action=review
> 
> > LayoutTests/platform/mac/accessibility/selection-initial.html:88
> > +        //
> 
> This should just be one line I think
> 
> > LayoutTests/platform/mac/accessibility/selection-initial.html:131
> > +        //
> 
> ditto
> 
> > Source/WebCore/page/EventHandler.cpp:3049
> > +static void setKeyboardSelection(Frame& frame, SelectionDirection direction)
> 
> should this be setInitialKeyboardSelection
> 
> > Source/WebCore/page/EventHandler.cpp:3052
> > +
> 
> you can remove this empty line
> 
> > Source/WebCore/ChangeLog:10
> > +        Added function to set initial selection more gracefully. Specifically, when accessibility has the need to set initial selection,
> 
> move this explanation above the line about "No New Tests'
> 
> This should say something like
> "Support the case where we want to set the initial selection and there's already a focused element."
> The current wording is a bit backward in terms of importance

Addressed all feedback and committed manually. Thanks.
Comment 6 Samuel White 2014-04-09 14:15:15 PDT
http://trac.webkit.org/changeset/167033