It keeps expanding the selection each time it's called, which doesn't match NSTextView, and makes no sense in general.
Created attachment 103505 [details] proposed fix
It's not very easy to hit this manually. The only way I know is by adding a key binding which calls this method to ~/Library/Keybindings/DefaultKeyBinding.dict.
Comment on attachment 103505 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=103505&action=review > Source/WebCore/editing/VisibleSelection.cpp:298 > if (isEndOfDocument(start) || (isEndOfLine(start) && !isStartOfLine(start) && !isEndOfParagraph(start))) > side = LeftWordIfOnBoundary; Should we do the same here? > Source/WebCore/editing/VisibleSelection.cpp:306 > + side = RightWordIfOnBoundary; > + if (isEndOfDocument(originalEnd) || (isEndOfLine(originalEnd) && !isStartOfLine(originalEnd) && !isEndOfParagraph(originalEnd))) > + side = LeftWordIfOnBoundary; > + } else > side = LeftWordIfOnBoundary; These enum values should be renamed to PreviousWordIfOnBoundary and NextWordIfOnBoundary (i.e. left/right are reversed for RTL text as is). > LayoutTests/editing/execCommand/select-word.html:1 > +<p>Test behavior of SelectWord editor command. It should be compatible with AppKit's selectWord:.</p> Nit: missing DOCTYPE also ":." > LayoutTests/editing/execCommand/select-word.html:30 > + [[3, 3], " "], Really? Shouldn't we select "123" in this case?
Comment on attachment 103505 [details] proposed fix Attachment 103505 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9333911 New failing tests: editing/selection/shift-click.html editing/selection/extend-after-mouse-selection.html editing/selection/5195166-1.html
Comment on attachment 103505 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=103505&action=review >> Source/WebCore/editing/VisibleSelection.cpp:298 >> side = LeftWordIfOnBoundary; > > Should we do the same here? I don't see the same kind of problem with start, so probably not. However, I extended the test to check that, and found some other problems that I'd like to address. >> Source/WebCore/editing/VisibleSelection.cpp:306 >> side = LeftWordIfOnBoundary; > > These enum values should be renamed to PreviousWordIfOnBoundary and NextWordIfOnBoundary (i.e. left/right are reversed for RTL text as is). Yup, but I don't think that it's something for this patch. >> LayoutTests/editing/execCommand/select-word.html:1 >> +<p>Test behavior of SelectWord editor command. It should be compatible with AppKit's selectWord:.</p> > > Nit: missing DOCTYPE also ":." I don't see a reason to add DOCTYPE (there's no <html> either, for that matter). The colon is not a typo, but I expanded that to -[NSTextView selectWord:] for clarity. >> LayoutTests/editing/execCommand/select-word.html:30 >> + [[3, 3], " "], > > Really? Shouldn't we select "123" in this case? Ugh. Turns out that NSTextView doesn't seem to have a well specified behavior. If you position the insertion point with arrow keys, it selects the space, and if you position it with a mouse, it selects preceding text. I've filed <rdar://problem/9931134> to find out what the correct behavior is.
(In reply to comment #5) > >> Source/WebCore/editing/VisibleSelection.cpp:306 > >> side = LeftWordIfOnBoundary; > > > > These enum values should be renamed to PreviousWordIfOnBoundary and NextWordIfOnBoundary (i.e. left/right are reversed for RTL text as is). > > Yup, but I don't think that it's something for this patch. Agreed. > >> LayoutTests/editing/execCommand/select-word.html:30 > >> + [[3, 3], " "], > > > > Really? Shouldn't we select "123" in this case? > > Ugh. Turns out that NSTextView doesn't seem to have a well specified behavior. If you position the insertion point with arrow keys, it selects the space, and if you position it with a mouse, it selects preceding text. I've filed <rdar://problem/9931134> to find out what the correct behavior is. We should also test what other browsers do.