Web Inspector: Unhandled exception when moving cursor mid-token after…

… receiving CSS property name completions

https://bugs.webkit.org/show_bug.cgi?id=234393
<rdar://problem/86578732>

Reviewed by Patrick Angle.

Source/WebInspectorUI:

A faulty check for mid-token completions in `WI.CSSKeywordCompletions.forPartialPropertyName()`, which are still
unsupported, prevented an early return and completions were provided unexpectedly. This had knock-on effects in
`WI.SpreadsheetTextField` which is not set up to handle cases where the caret is placed within the completion query.
Calculating the adjusted caret position could return a negative index and throw an unhandled exception.

Web Inspector does not currently explicitly support mid-token completions. See https://webkit.org/b/227157

The implementation of fuzzy matching for CSS completions in https://webkit.org/b/234092
means that, unhindered, the completion provider for CSS property names _can_ return mid-token completions.
Typing a query like `margin`, then moving the caret to the beginning and correcting to `s|margin` will return
completions like `[s]croll-[margin]`. Accepting the completion results in a malformed `SpreadsheetTextField.value`
by concatenation within the prefix itself. As the user types mid-token, the prefix becomes ambiguous.

Fixing the condition in `WI.CSSKeywordCompletions.forPartialPropertyName()` now inhibits unintentional
mid-token completions when fuzzy matching is enabled.

* UserInterface/Models/CSSKeywordCompletions.js:
(WI.CSSKeywordCompletions.forPartialPropertyName):
* UserInterface/Views/SpreadsheetTextField.js:
(WI.SpreadsheetTextField.prototype._showSuggestionsView):

LayoutTests:

* inspector/unit-tests/css-keyword-completions-expected.txt:
* inspector/unit-tests/css-keyword-completions.html:



Canonical link: https://commits.webkit.org/245965@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287934 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,14 @@
+2022-01-12  Razvan Caliman  <rcaliman@apple.com>
+
+        Web Inspector: Unhandled exception when moving cursor mid-token after receiving CSS property name completions
+        https://bugs.webkit.org/show_bug.cgi?id=234393
+        <rdar://problem/86578732>
+
+        Reviewed by Patrick Angle.
+
+        * inspector/unit-tests/css-keyword-completions-expected.txt:
+        * inspector/unit-tests/css-keyword-completions.html:
+
 2022-01-12  Robert Jenner  <Jenner@apple.com>
 
         [AppleWin] Some fast/shadow-dom/fullscreen-in-* tests are crashing after r287698

LayoutTests/inspector/unit-tests/css-keyword-completions-expected.txt

@@ -15,6 +15,10 @@ PASS: All expected completions were present.
 -- Running test case: WI.CSSKeywordCompletions.forPartialPropertyName.multipleCharacterMatches
 PASS: All expected completions were present.
 
+-- Running test case: WI.CSSKeywordCompletions.forPartialPropertyName.midTokenNoCompletions
+PASS: Expected exactly 0 completion results.
+PASS: All expected completions were present.
+
 -- Running test case: WI.CSSKeywordCompletions.forPartialPropertyValue.EmptyTop
 PASS: Expected result prefix to be ""
 PASS: All expected completions were present.

LayoutTests/inspector/unit-tests/css-keyword-completions.html

@@ -7,7 +7,7 @@
 {
     let suite = InspectorTest.createSyncSuite("WI.CSSKeywordCompletions");
 
-    function addTestForPartialPropertyName({name, description, text, allowEmptyPrefix, expectedCompletions, expectedCompletionCount}) {
+    function addTestForPartialPropertyName({name, description, text, caretPosition, allowEmptyPrefix, expectedCompletions, expectedCompletionCount}) {
         suite.addTestCase({
             name,
             description,
@@ -17,7 +17,7 @@
                 expectedCompletionCount ??= -1;
 
                 // FIXME: <webkit.org/b/227157> Styles: Support completions mid-token.
-                let caretPosition = text.length;
+                caretPosition ??= text.length;
                 let completionResults = WI.CSSKeywordCompletions.forPartialPropertyName(text, {caretPosition, allowEmptyPrefix});
 
                 if (expectedCompletionCount >= 0)
@@ -66,6 +66,15 @@
         expectedCompletions: ["range"],
     });
 
+    // FIXME: This test will fail after addressing <webkit.org/b/227157> Styles: Support completions mid-token.
+    addTestForPartialPropertyName({
+        name: "WI.CSSKeywordCompletions.forPartialPropertyName.midTokenNoCompletions",
+        description: "Test that completions are not provided when the caret is positioned within the token.",
+        text: "margin",
+        caretPosition: 1,
+        expectedCompletionCount: 0,
+    });
+
     function addTestForPartialPropertyValue({name, description, propertyName, text, caretPosition, expectedPrefix, expectedCompletions, expectedCompletionCount, additionalFunctionValueCompletionsProvider}) {
         suite.addTestCase({
             name,

Source/WebInspectorUI/ChangeLog

@@ -1,3 +1,32 @@
+2022-01-12  Razvan Caliman  <rcaliman@apple.com>
+
+        Web Inspector: Unhandled exception when moving cursor mid-token after receiving CSS property name completions
+        https://bugs.webkit.org/show_bug.cgi?id=234393
+        <rdar://problem/86578732>
+
+        Reviewed by Patrick Angle.
+
+        A faulty check for mid-token completions in `WI.CSSKeywordCompletions.forPartialPropertyName()`, which are still
+        unsupported, prevented an early return and completions were provided unexpectedly. This had knock-on effects in
+        `WI.SpreadsheetTextField` which is not set up to handle cases where the caret is placed within the completion query.
+        Calculating the adjusted caret position could return a negative index and throw an unhandled exception.
+
+        Web Inspector does not currently explicitly support mid-token completions. See https://webkit.org/b/227157
+
+        The implementation of fuzzy matching for CSS completions in https://webkit.org/b/234092
+        means that, unhindered, the completion provider for CSS property names _can_ return mid-token completions.
+        Typing a query like `margin`, then moving the caret to the beginning and correcting to `s|margin` will return
+        completions like `[s]croll-[margin]`. Accepting the completion results in a malformed `SpreadsheetTextField.value`
+        by concatenation within the prefix itself. As the user types mid-token, the prefix becomes ambiguous.
+
+        Fixing the condition in `WI.CSSKeywordCompletions.forPartialPropertyName()` now inhibits unintentional
+        mid-token completions when fuzzy matching is enabled.
+
+        * UserInterface/Models/CSSKeywordCompletions.js:
+        (WI.CSSKeywordCompletions.forPartialPropertyName):
+        * UserInterface/Views/SpreadsheetTextField.js:
+        (WI.SpreadsheetTextField.prototype._showSuggestionsView):
+
 2022-01-11  Nikita Vasilyev  <nvasilyev@apple.com>
 
         REGRESSION (r283723): Web Inspector: CSS declarations unexpectedly removed when editing property value

Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js

@@ -33,12 +33,11 @@ WI.CSSKeywordCompletions = {};
 
 WI.CSSKeywordCompletions.forPartialPropertyName = function(text, {caretPosition, allowEmptyPrefix, useFuzzyMatching} = {})
 {
-    caretPosition ??= text.length;
     allowEmptyPrefix ??= false;
 
     // FIXME: <webkit.org/b/227157> Styles: Support completions mid-token.
-    if (caretPosition !== caretPosition)
-        return {prefix: text, completions: []};
+    if (caretPosition !== text.length)
+        return {prefix: "", completions: []};
 
     if (!text.length && allowEmptyPrefix)
         return {prefix: text, completions: WI.cssManager.propertyNameCompletions.values};

Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js

@@ -478,6 +478,8 @@ WI.SpreadsheetTextField = class SpreadsheetTextField
         // border: 1px solid ro|
         //                   rosybrown
         //                   royalblue
+        //
+        // FIXME: Account for the caret being within the token when fixing <webkit.org/b/227157> Styles: Support completions mid-token.
         let adjustedCaretPosition = this._getCaretPosition() - this._completionPrefix.length;
         let caretRect = this._getCaretRect(adjustedCaretPosition);