- Revision
- 287934
- Author
- rcali...@apple.com
- Date
- 2022-01-12 10:49:16 -0800 (Wed, 12 Jan 2022)
Log Message
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:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (287933 => 287934)
--- trunk/LayoutTests/ChangeLog 2022-01-12 18:44:53 UTC (rev 287933)
+++ trunk/LayoutTests/ChangeLog 2022-01-12 18:49:16 UTC (rev 287934)
@@ -1,3 +1,14 @@
+2022-01-12 Razvan Caliman <rcali...@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 <jen...@apple.com>
[AppleWin] Some fast/shadow-dom/fullscreen-in-* tests are crashing after r287698
Modified: trunk/LayoutTests/inspector/unit-tests/css-keyword-completions-expected.txt (287933 => 287934)
--- trunk/LayoutTests/inspector/unit-tests/css-keyword-completions-expected.txt 2022-01-12 18:44:53 UTC (rev 287933)
+++ trunk/LayoutTests/inspector/unit-tests/css-keyword-completions-expected.txt 2022-01-12 18:49:16 UTC (rev 287934)
@@ -15,6 +15,10 @@
-- 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.
Modified: trunk/LayoutTests/inspector/unit-tests/css-keyword-completions.html (287933 => 287934)
--- trunk/LayoutTests/inspector/unit-tests/css-keyword-completions.html 2022-01-12 18:44:53 UTC (rev 287933)
+++ trunk/LayoutTests/inspector/unit-tests/css-keyword-completions.html 2022-01-12 18:49:16 UTC (rev 287934)
@@ -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,
Modified: trunk/Source/WebInspectorUI/ChangeLog (287933 => 287934)
--- trunk/Source/WebInspectorUI/ChangeLog 2022-01-12 18:44:53 UTC (rev 287933)
+++ trunk/Source/WebInspectorUI/ChangeLog 2022-01-12 18:49:16 UTC (rev 287934)
@@ -1,3 +1,32 @@
+2022-01-12 Razvan Caliman <rcali...@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 <nvasil...@apple.com>
REGRESSION (r283723): Web Inspector: CSS declarations unexpectedly removed when editing property value
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js (287933 => 287934)
--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js 2022-01-12 18:44:53 UTC (rev 287933)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSKeywordCompletions.js 2022-01-12 18:49:16 UTC (rev 287934)
@@ -33,12 +33,11 @@
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};
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js (287933 => 287934)
--- trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js 2022-01-12 18:44:53 UTC (rev 287933)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js 2022-01-12 18:49:16 UTC (rev 287934)
@@ -478,6 +478,8 @@
// 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);
