Bug 164519 – Word transformations apply to a single word instead of entire selection if selection is made by triple-clicking

• Status: NEW
• Component: HTML Editing

<rdar://problem/27487147>

To reproduce:
Navigate to the URL, triple-click the sentence to select it, then control-click and choose Transformations > Make Upper Case

Result:
Only the word in which the control-click occurred is transformed and stays selected

Note:
The implementation of word transformations begins with performing the SelectWord command. The intent is to expand the selection to encompass whole words (that’s how the command behaves in NSTextView). When made with a triple-click, the selection’s base and extent are the same (the position of the click). It then expands to a word around that position. This happens in the VisibleSelection::setStartAndEndFromBaseAndExtentRespectingGranularity call from VisibleSelection::validate. It seems like we need a different code path for expanding to a desired granularity.

[Darin Adler]

The function VisibleSelection::expandUsingGranularity exists; is that relevant?

[Darin Adler]

And SelectionController::expandUsingGranularity.

(In reply to comment #1)
> The function VisibleSelection::expandUsingGranularity exists; is that
> relevant?

Yes, VisibleSelection::expandUsingGranularity is the one calling VisibleSelection::validate in this case. Here’s the call stack:

WebCore::VisibleSelection::setStartAndEndFromBaseAndExtentRespectingGranularity(WebCore::TextGranularity) at Source/WebCore/editing/VisibleSelection.cpp:272
WebCore::VisibleSelection::validate(WebCore::TextGranularity) at Source/WebCore/editing/VisibleSelection.cpp:422
WebCore::VisibleSelection::expandUsingGranularity(WebCore::TextGranularity) at Source/WebCore/editing/VisibleSelection.cpp:198
WebCore::expandSelectionToGranularity(WebCore::Frame&, WebCore::TextGranularity) at Source/WebCore/editing/EditorCommand.cpp:177
WebCore::executeSelectWord(WebCore::Frame&, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&) at Source/WebCore/editing/EditorCommand.cpp:1015
WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const at Source/WebCore/editing/EditorCommand.cpp:1777
::-[WebHTMLView executeCoreCommandBySelector:](SEL) at Source/WebKit/mac/WebView/WebHTMLView.mm:2954
::-[WebHTMLView selectWord:](id) at Source/WebKit/mac/WebView/WebHTMLView.mm:3056
::-[WebHTMLView _changeWordCaseWithSelector:](SEL) at Source/WebKit/mac/WebView/WebHTMLView.mm:5824
::-[WebHTMLView uppercaseWord:](id) at Source/WebKit/mac/WebView/WebHTMLView.mm:5835

(In reply to comment #2)
> And SelectionController::expandUsingGranularity.

I don’t see that one in TOT.

[Darin Adler]

So I guess you are saying that this is a bug in WebCore::expandSelectionToGranularity/VisibleSelection::expandUsingGranularity.

(In reply to comment #4)
> So I guess you are saying that this is a bug in
> WebCore::expandSelectionToGranularity/VisibleSelection::
> expandUsingGranularity.

I am saying that those functions’ behavior doesn’t meet the requirements of word transformations. I don’t know how changing their behavior so that they never contract the selection will affect other uses. I also don’t know exactly how to do that. I tried, naively, to make VisibleSelection::expandUsingGranularity set the base and the extent to the start and end before calling validate, and then restore them, but that wasn’t quite right: it produced word selections that included the trailing space after the word.

[Darin Adler]

I think the trick is to make test cases first that express the behavior we want. I don’t think it will be terribly difficult to get the implementation right.

Certainly the name "expand selection to granularity" implies the behavior of never contracting selections, so if we need different behavior for some purposes, those should be in functions with different names!

[Darin Adler]

All those special cases in the WordGranularity section of setStartAndEndFromBaseAndExtentRespectingGranularity are probably the source of the difficulty with the trailing space after the word.

You mentioned setting the base and extent to the start and end before calling validate. Was the code you tried respecting m_baseIsFirst?

(In reply to comment #7)
> All those special cases in the WordGranularity section of
> setStartAndEndFromBaseAndExtentRespectingGranularity are probably the source
> of the difficulty with the trailing space after the word.
> 
> You mentioned setting the base and extent to the start and end before
> calling validate. Was the code you tried respecting m_baseIsFirst?

Yes, it was naïve but not that naïve. I will take a deeper look and try to understand what’s going on.

[Darin Adler]

Perhaps we should change things so the granularity is recorded alongside m_start and m_end. Then a call to expandUsingGranularity targeting any smaller granularity would automatically be a no-op.

This would not work for line vs. sentence granularity, but should work fine for character, word, sentence/line, paragraph, document.