164519 2016-11-08 10:13:28 -0800 Word transformations apply to a single word instead of entire selection if selection is made by triple-clicking 2018-07-12 20:21:20 -0700 1 1 1 Unclassified WebKit HTML Editing Other Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=187605 data:text/html,<div%20contenteditable>Triple-click%20this%20sentence,%20then%20control-click%20and%20choose%20Transformations%20&gt;%20Make%20Upper%20Case InRadar P2 Normal --- 1 mitz webkit-unassigned ap darin oldest_to_newest 1249077 0 mitz 2016-11-08 10:13:28 -0800 <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. 1250845 1 darin 2016-11-13 11:26:57 -0800 The function VisibleSelection::expandUsingGranularity exists; is that relevant? 1250846 2 darin 2016-11-13 11:27:11 -0800 And SelectionController::expandUsingGranularity. 1250853 3 mitz 2016-11-13 11:36:18 -0800 (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. 1250855 4 darin 2016-11-13 11:45:25 -0800 So I guess you are saying that this is a bug in WebCore::expandSelectionToGranularity/VisibleSelection::expandUsingGranularity. 1250858 5 mitz 2016-11-13 11:51:12 -0800 (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. 1250882 6 darin 2016-11-13 16:53:10 -0800 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! 1250884 7 darin 2016-11-13 16:56:32 -0800 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? 1250885 8 mitz 2016-11-13 16:59:49 -0800 (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. 1250886 9 darin 2016-11-13 17:03:26 -0800 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.