Skip to content
Permalink
Browse files
Web Inspector: CSS autocomplete: suggestion hint should be the most c…
…ommonly used property and not the alphabetically first one

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

Reviewed by Patrick Angle.

* Source/WebInspectorUI/UserInterface/Models/CSSProperty.js:
(WI.CSSProperty):
(WI.CSSProperty.sortByPropertyNameUsageCount): Added.
(WI.CSSProperty._initializePropertyNameCounts): Added.
(WI.CSSProperty.prototype.update):
(WI.CSSProperty.prototype.remove):
(WI.CSSProperty.prototype.set name):
(WI.CSSProperty.prototype._updateName): Added.
Every time a `WI.CSSProperty` is created/updated, increment/decrement the count of that property's
name in the global map of property name counts. When showing CSS completions for property names,
initially focus the property with the highest count (but only if the count is over 100).

* Source/WebInspectorUI/UserInterface/Base/ObjectStore.js:
(WI.ObjectStore._open):
(WI.ObjectStore.async getAllKeys): Added.
Increment the `version` to add a new `WI.ObjectStore` for CSS property name counts.

* Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:
(WI.SpreadsheetStyleProperty.prototype.spreadsheetTextFieldInitialCompletionIndex): Added.
* Source/WebInspectorUI/UserInterface/Views/SpreadsheetTextField.js:
(WI.SpreadsheetTextField.prototype._updateCompletions):
Add `delegate` methods to adjust the initial `selectedIndex` whenever completions are updated.

* Source/WebInspectorUI/UserInterface/Views/CompletionSuggestionsView.js:
(WI.CompletionSuggestionsView.prototype.set selectedIndex):
(WI.CompletionSuggestionsView.prototype.selectNext):
(WI.CompletionSuggestionsView.prototype.selectPrevious):
Make sure the `delegate` is informed whenever the `selectedIndex` changes, not just when selecting
the next/previous item.

* Source/WebInspectorUI/UserInterface/Base/Setting.js:
* Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:
(WI.SettingsTabContentView.prototype._createExperimentalSettingsView):
* Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js:
Add an experimental setting to control this behavior.

* Source/WebInspectorUI/UserInterface/Base/Utilities.js:
(Array.prototype.minIndex): Added.
Add a utility method for finding the index of the smallest item in an array.

* LayoutTests/inspector/css/css-property.html:
* LayoutTests/inspector/css/css-property-expected.txt:
* LayoutTests/inspector/unit-tests/array-utilities.html:
* LayoutTests/inspector/unit-tests/array-utilities-expected.txt:

Canonical link: https://commits.webkit.org/250994@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294862 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
dcrousso committed May 26, 2022
1 parent 2c26e24 commit 411a48c8e636110ad7cb9184edd2b437a9ec4521
Showing 13 changed files with 222 additions and 17 deletions.
@@ -2,6 +2,31 @@ Testing methods of CSSProperty.


== Running test suite: CSSProperty
-- Running test case: CSSProperty.NameCount
PASS: "display" should have at least 2 counts.
PASS: "background-repeat" should have at least 1 count.
PASS: "background-repeat-x" should not be counted.
PASS: "background-repeat-y" should not be counted.
PASS: "background-repeat-invalid" should not be counted.
[
"Fake Property 200 Seen",
"Fake Property 150 Seen",
"Fake Property 101 Seen",
"Fake Property 100 Seen",
"Fake Property 99 Unseen",
"Fake Property 99 Seen",
"Fake Property 1 Unseen",
"Fake Property 1 Seen",
"Fake Property 100 Unseen",
"Fake Property 0 Unseen",
"Fake Property 0 Seen",
"Fake Property 101 Unseen",
"Fake Property 150 Unseen",
"Fake Property 50 Unseen",
"Fake Property 50 Seen",
"Fake Property 200 Unseen"
]

-- Running test case: CSSProperty.prototype.get valid
PASS: "background-repeat" is a valid property.
PASS: "background-repeat-x" is an invalid property.
@@ -8,6 +8,27 @@

let suite = InspectorTest.createAsyncSuite("CSSProperty");

suite.addTestCase({
name: "CSSProperty.NameCount",
description: "Ensure that the name counts are being tracked.",
async test() {
InspectorTest.expectGreaterThanOrEqual(WI.CSSProperty._cachedNameCounts["display"], 2, `"display" should have at least 2 counts.`);
InspectorTest.expectGreaterThanOrEqual(WI.CSSProperty._cachedNameCounts["background-repeat"], 1, `"background-repeat" should have at least 1 count.`);
InspectorTest.expectThat(!("background-repeat-x" in WI.CSSProperty._cachedNameCounts), `"background-repeat-x" should not be counted.`);
InspectorTest.expectThat(!("background-repeat-y" in WI.CSSProperty._cachedNameCounts), `"background-repeat-y" should not be counted.`);
InspectorTest.expectThat(!("background-repeat-invalid" in WI.CSSProperty._cachedNameCounts), `"background-repeat-invalid" should not be counted.`);

const counts = [99, 1, 100, 0, 101, 150, 50, 200];
let seenProperty = (count) => `Fake Property ${count} Seen`;
let unseenProperty = (count) => `Fake Property ${count} Unseen`;
for (const count of counts)
WI.CSSProperty._cachedNameCounts[seenProperty(count)] = count;
InspectorTest.json(counts.flatMap((count) => [unseenProperty(count), seenProperty(count)]).sort(WI.CSSProperty.sortByPropertyNameUsageCount))
for (const count of counts)
delete WI.CSSProperty._cachedNameCounts[seenProperty(count)];
}
});

suite.addTestCase({
name: "CSSProperty.prototype.get valid",
description: "Ensure valid, invalid, and internal-only have correct valid state.",
@@ -250,6 +271,8 @@

<style>
div#x {
display: initial;
display: initial;
background-repeat: repeat;
background-repeat-x: repeat;
background-repeat-invalid: repeat;
@@ -1,5 +1,14 @@

== Running test suite: ArrayUtilities
-- Running test case: Array.prototype.minIndex
PASS: minIndex of an empty array should be 0.
PASS: minIndex of an array with one item should be the index of the item.
PASS: minIndex of an increasing array should be 0.
PASS: minIndex of an decreasing array should be length - 1.
PASS: minIndex of a mixed array should be the index of the smallest item.
PASS: minIndex of an array with duplicates should be the index of the smallest item.
PASS: minIndex with a comparator of a mixed array should be the index of the smallest item.

-- Running test case: Array.prototype.lowerBound
PASS: lowerBound of a value before the first value should produce the first index.
PASS: lowerBound of a value in the list should return the value's index.
@@ -7,6 +7,24 @@
{
let suite = InspectorTest.createSyncSuite("ArrayUtilities");

suite.addTestCase({
name: "Array.prototype.minIndex",
test() {
InspectorTest.expectEqual([].minIndex(), 0, "minIndex of an empty array should be 0.");
InspectorTest.expectEqual([1].minIndex(), 0, "minIndex of an array with one item should be the index of the item.");
InspectorTest.expectEqual([1, 2, 3, 4].minIndex(), 0, "minIndex of an increasing array should be 0.");
InspectorTest.expectEqual([4, 3, 2, 1].minIndex(), 3, "minIndex of an decreasing array should be length - 1.");
InspectorTest.expectEqual([3, 4, 1, 2].minIndex(), 2, "minIndex of a mixed array should be the index of the smallest item.");
InspectorTest.expectEqual([3, 1, 1, 3].minIndex(), 1, "minIndex of an array with duplicates should be the index of the smallest item.");

let objs = [{value: 3}, {value: 4}, {value: 1}, {value: 2}];
let comparator = (a, b) => a.value - b.value;
InspectorTest.expectEqual(objs.minIndex(comparator), 2, "minIndex with a comparator of a mixed array should be the index of the smallest item.");

return true;
}
});

suite.addTestCase({
name: "Array.prototype.lowerBound",
test() {
@@ -1503,6 +1503,7 @@ localizedStrings["Subject"] = "Subject";
localizedStrings["Subscript @ Font Details Sidebar Property Value"] = "Subscript";
/* A submenu item of 'Break On' that breaks (pauses) before child DOM node is modified */
localizedStrings["Subtree Modified @ DOM Breakpoint"] = "Subtree Modified";
localizedStrings["Suggest property names based on usage"] = "Suggest property names based on usage";
localizedStrings["Summary"] = "Summary";
/* Property value for `font-variant-position: super`. */
localizedStrings["Superscript @ Font Details Sidebar Property Value"] = "Superscript";
@@ -67,7 +67,7 @@ WI.ObjectStore = class ObjectStore

WI.ObjectStore._databaseCallbacks = [callback];

const version = 5; // Increment this for every edit to `WI.objectStores`.
const version = 6; // Increment this for every edit to `WI.objectStores`.

let databaseRequest = window.indexedDB.open(WI.ObjectStore._databaseName, version);
databaseRequest.addEventListener("upgradeneeded", (event) => {
@@ -135,6 +135,14 @@ WI.ObjectStore = class ObjectStore
return this._operation("readonly", (objectStore) => objectStore.getAll(...args));
}

async getAllKeys(...args)
{
if (!WI.ObjectStore.supported())
return [];

return this._operation("readonly", (objectStore) => objectStore.getAllKeys(...args));
}

async put(...args)
{
if (!WI.ObjectStore.supported())
@@ -248,11 +256,23 @@ WI.ObjectStore.toJSONSymbol = Symbol("ObjectStore-toJSON");

// Be sure to update the `version` above when making changes.
WI.objectStores = {
general: new WI.ObjectStore("general"),
// Version 1
audits: new WI.ObjectStore("audit-manager-tests", {keyPath: "__id", autoIncrement: true}),

// Version 2
breakpoints: new WI.ObjectStore("debugger-breakpoints", {keyPath: "__id"}),

// Version 3
domBreakpoints: new WI.ObjectStore("dom-debugger-dom-breakpoints", {keyPath: "__id"}),
eventBreakpoints: new WI.ObjectStore("dom-debugger-event-breakpoints", {keyPath: "__id"}),
urlBreakpoints: new WI.ObjectStore("dom-debugger-url-breakpoints", {keyPath: "__id"}),

// Version 4
localResourceOverrides: new WI.ObjectStore("local-resource-overrides", {keyPath: "__id"}),

// Version 5
general: new WI.ObjectStore("general"),

// Version 6
cssPropertyNameCounts: new WI.ObjectStore("css-property-name-counts"),
};
@@ -232,6 +232,7 @@ WI.settings = {
experimentalEnableStylesJumpToVariableDeclaration: new WI.Setting("experimental-styles-jump-to-variable-declaration", false),
experimentalAllowInspectingInspector: new WI.Setting("experimental-allow-inspecting-inspector", false),
experimentalCSSCompletionFuzzyMatching: new WI.Setting("experimental-css-completion-fuzzy-matching", true),
experimentalCSSSortPropertyNameAutocompletionByUsage: new WI.Setting("experimental-css-sort-property-name-autocompletion-by-usage", false),
experimentalShowScreenshotsTimeline: new WI.Setting("experimental-show-screenshots-timeline", false),

// Protocol
@@ -1592,6 +1592,25 @@ function simpleGlobStringToRegExp(globString, regExpFlags)
return new RegExp(regexString, regExpFlags);
}

Object.defineProperty(Array.prototype, "minIndex",
{
value(comparator)
{
function defaultComparator(a, b)
{
return a - b;
}
comparator = comparator || defaultComparator;

let minIndex = 0;
for (let i = 1; i < this.length; ++i) {
if (comparator(this[minIndex], this[i]) > 0)
minIndex = i;
}
return minIndex;
},
});

Object.defineProperty(Array.prototype, "lowerBound",
{
// Return index of the leftmost element that is equal or greater
@@ -27,6 +27,8 @@ WI.CSSProperty = class CSSProperty extends WI.Object
{
constructor(index, text, name, value, priority, enabled, overridden, implicit, anonymous, valid, styleSheetTextRange)
{
WI.CSSProperty._initializePropertyNameCounts();

super();

this._ownerStyle = null;
@@ -83,6 +85,49 @@ WI.CSSProperty = class CSSProperty extends WI.Object
return names;
}

static sortByPropertyNameUsageCount(propertyNameA, propertyNameB)
{
let countA = WI.CSSProperty._cachedNameCounts[propertyNameA];
let countB = WI.CSSProperty._cachedNameCounts[propertyNameB];

const minimumCount = 100;
let validA = countA >= minimumCount;
let validB = countB >= minimumCount;

if (validA && !validB)
return -1;
if (!validA && validB)
return 1;

if (validA && validB) {
if (countA !== countB)
return countB - countA;

let canonicalPropertyNameA = WI.cssManager.canonicalNameForPropertyName(propertyNameA);
let canonicalPropertyNameB = WI.cssManager.canonicalNameForPropertyName(propertyNameB);
if (canonicalPropertyNameA !== propertyNameA || canonicalPropertyNameB !== propertyNameB)
return WI.CSSProperty.sortByPropertyNameUsageCount(canonicalPropertyNameA, canonicalPropertyNameB);
}

return 0;
}

static _initializePropertyNameCounts()
{
if (WI.CSSProperty._cachedNameCounts)
return;

WI.CSSProperty._cachedNameCounts = {};

WI.objectStores.cssPropertyNameCounts.getAllKeys().then((propertyNames) => {
for (let propertyName of propertyNames) {
WI.objectStores.cssPropertyNameCounts.get(propertyName).then((storedCount) => {
WI.CSSProperty._cachedNameCounts[propertyName] = (WI.CSSProperty._cachedNameCounts[propertyName] || 0) + storedCount;
});
}
});
}

// Public

get ownerStyle()
@@ -140,7 +185,6 @@ WI.CSSProperty = class CSSProperty extends WI.Object
this._overridingProperty = null;

this._text = text;
this._name = name;
this._rawValue = value;
this._value = undefined;
this._priority = priority;
@@ -163,6 +207,7 @@ WI.CSSProperty = class CSSProperty extends WI.Object
this._isShorthand = undefined;
this._shorthandPropertyNames = undefined;

this._updateName(name);
this._relatedShorthandProperty = null;
this._relatedLonghandProperties = [];

@@ -180,7 +225,7 @@ WI.CSSProperty = class CSSProperty extends WI.Object
this._markModified();

// Setting name or value to an empty string removes the entire CSSProperty.
this._name = "";
this._updateName("");
const forceRemove = true;
this._updateStyleText(forceRemove);
}
@@ -268,7 +313,7 @@ WI.CSSProperty = class CSSProperty extends WI.Object
}

this._markModified();
this._name = name;
this._updateName(name);
this._updateStyleText();
}

@@ -503,6 +548,42 @@ WI.CSSProperty = class CSSProperty extends WI.Object

// Private

_updateName(name)
{
if (name === this._name)
return;

let changeCount = (propertyName, delta) => {
if (!propertyName || this._implicit || this._anonymous || !this._enabled)
return;

let oldCount = WI.CSSProperty._cachedNameCounts[propertyName];

// Allow property counts to be updated if the property name has already been counted before.
// This can happen when inspecting a device that has different CSS properties enabled.
if (isNaN(oldCount) && !WI.cssManager.propertyNameCompletions.isValidPropertyName(propertyName))
return;

console.assert(delta > 0 || oldCount >= delta, oldCount, delta);
let newCount = Math.max(0, (oldCount || 0) + delta);
WI.CSSProperty._cachedNameCounts[propertyName] = newCount;

WI.objectStores.cssPropertyNameCounts.get(propertyName).then((storedCount) => {
console.assert(delta > 0 || storedCount >= delta, storedCount, delta);
storedCount = Math.max(0, (storedCount || 0) + delta);
WI.CSSProperty._cachedNameCounts[propertyName] = storedCount;
WI.objectStores.cssPropertyNameCounts.put(storedCount, propertyName);
});

if (propertyName !== this.canonicalName)
changeCount(this.canonicalName, delta);
};

changeCount(this._name, -1);
this._name = name;
changeCount(this._name, 1);
}

_updateStyleText(forceRemove = false)
{
let text = "";
@@ -537,6 +618,8 @@ WI.CSSProperty = class CSSProperty extends WI.Object
}
};

WI.CSSProperty._cachedNameCounts = null;

WI.CSSProperty.Event = {
Changed: "css-property-changed",
ModifiedChanged: "css-property-modified-changed",
@@ -74,11 +74,13 @@ WI.CompletionSuggestionsView = class CompletionSuggestionsView extends WI.Object
this._selectedIndex = index;

selectedItemElement = this._selectedItemElement;
if (!selectedItemElement)
return;
if (selectedItemElement) {
selectedItemElement.classList.add("selected");
selectedItemElement.scrollIntoViewIfNeeded(false);
}

selectedItemElement.classList.add("selected");
selectedItemElement.scrollIntoViewIfNeeded(false);
if (this._completions[this._selectedIndex])
this._delegate?.completionSuggestionsSelectedCompletion?.(this, this.getCompletionText(this._completions[this._selectedIndex]));
}

selectNext()
@@ -89,9 +91,6 @@ WI.CompletionSuggestionsView = class CompletionSuggestionsView extends WI.Object
this.selectedIndex = 0;
else
++this.selectedIndex;

if (this._completions[this.selectedIndex])
this._delegate?.completionSuggestionsSelectedCompletion?.(this, this.getCompletionText(this._completions[this.selectedIndex]));
}

selectPrevious()
@@ -100,9 +99,6 @@ WI.CompletionSuggestionsView = class CompletionSuggestionsView extends WI.Object
this.selectedIndex = this._containerElement.children.length - 1;
else
--this.selectedIndex;

if (this._completions[this.selectedIndex])
this._delegate?.completionSuggestionsSelectedCompletion?.(this, this.getCompletionText(this._completions[this.selectedIndex]));
}

isHandlingClickEvent()
@@ -392,6 +392,7 @@ WI.SettingsTabContentView = class SettingsTabContentView extends WI.TabContentVi
stylesGroup.addSetting(WI.settings.experimentalEnableStylesJumpToEffective, WI.UIString("Show jump to effective property button"));
stylesGroup.addSetting(WI.settings.experimentalEnableStylesJumpToVariableDeclaration, WI.UIString("Show jump to variable declaration button"));
stylesGroup.addSetting(WI.settings.experimentalCSSCompletionFuzzyMatching, WI.UIString("Use fuzzy matching for completion suggestions"));
stylesGroup.addSetting(WI.settings.experimentalCSSSortPropertyNameAutocompletionByUsage, WI.UIString("Suggest property names based on usage"));

experimentalSettingsView.addSeparator();
}

0 comments on commit 411a48c

Please sign in to comment.