Diff
Modified: trunk/LayoutTests/ChangeLog (245990 => 245991)
--- trunk/LayoutTests/ChangeLog 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/LayoutTests/ChangeLog 2019-05-31 22:53:40 UTC (rev 245991)
@@ -1,3 +1,16 @@
+2019-05-31 Nikita Vasilyev <[email protected]>
+
+ Web Inspector: CSS Changes: modifications aren't shared for rules that match multiple elements
+ https://bugs.webkit.org/show_bug.cgi?id=195264
+ <rdar://problem/48550023>
+
+ Reviewed by Devin Rousso.
+
+ Test arrays with repeating items for Array.diffArrays.
+
+ * inspector/unit-tests/array-utilities-expected.txt:
+ * inspector/unit-tests/array-utilities.html:
+
2019-05-31 Ryosuke Niwa <[email protected]>
iOS: Main frame should be scrollable when pinch zoomed or software keyboard is up
Modified: trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt (245990 => 245991)
--- trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt 2019-05-31 22:53:40 UTC (rev 245991)
@@ -81,9 +81,19 @@
["b","a"], ["a"] => [["b",-1],["a",0]]
["b","a"], ["a","c"] => [["b",-1],["a",0],["c",1]]
["b","a"], ["a","c"] => [["b",-1],["a",0],["c",1]]
-["b","a"], ["a","b"] => [["a",0],["b",0]]
+["b","a"], ["a","b"] => [["a",1],["b",0],["a",-1]]
["a","b","c"], ["a","d","c"] => [["a",0],["b",-1],["d",1],["c",0]]
+["a","b","c"], ["c","b","a"] => [["c",1],["b",1],["a",0],["b",-1],["c",-1]]
+Repeating items:
+["a"], ["a","a"] => [["a",0],["a",1]]
+["a","a"], ["a"] => [["a",0],["a",-1]]
+["a","a"], ["a","a"] => [["a",0],["a",0]]
+["b","a","b"], ["a","b","a"] => [["a",1],["b",0],["a",0],["b",-1]]
+["a","b","b","c"], ["c","b","b","b","a"] => [["a",-1],["c",1],["b",0],["b",0],["c",-1],["b",1],["a",1]]
+["a","b","b","b","c"], ["c","b","b","a"] => [["a",-1],["c",1],["b",0],["b",0],["b",-1],["c",-1],["a",1]]
+["a","a","b","b","c","c"], ["b","b","c","c","a","a"] => [["a",-1],["a",-1],["b",0],["b",0],["c",0],["c",0],["a",1],["a",1]]
+
-- Running test case: Array.prototype.lastValue
PASS: lastValue of a nonempty array should be the last value.
PASS: lastValue of an empty array should be undefined.
Modified: trunk/LayoutTests/inspector/unit-tests/array-utilities.html (245990 => 245991)
--- trunk/LayoutTests/inspector/unit-tests/array-utilities.html 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/LayoutTests/inspector/unit-tests/array-utilities.html 2019-05-31 22:53:40 UTC (rev 245991)
@@ -175,7 +175,17 @@
diff(["b", "a"], ["a", "c"]);
diff(["b", "a"], ["a", "b"]);
diff(["a", "b", "c"], ["a", "d", "c"]);
+ diff(["a", "b", "c"], ["c", "b", "a"]);
+ InspectorTest.log("\nRepeating items:");
+ diff(["a"], ["a", "a"]);
+ diff(["a", "a"], ["a"]);
+ diff(["a", "a"], ["a", "a"]);
+ diff(["b", "a", "b"], ["a", "b", "a"]);
+ diff(["a", "b", "b", "c"], ["c", "b", "b", "b", "a"]);
+ diff(["a", "b", "b", "b", "c"], ["c", "b", "b", "a"]);
+ diff(["a", "a", "b", "b", "c", "c"], ["b", "b", "c", "c", "a", "a"]);
+
return true;
}
});
Modified: trunk/Source/WebInspectorUI/ChangeLog (245990 => 245991)
--- trunk/Source/WebInspectorUI/ChangeLog 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/ChangeLog 2019-05-31 22:53:40 UTC (rev 245991)
@@ -1,3 +1,47 @@
+2019-05-31 Nikita Vasilyev <[email protected]>
+
+ Web Inspector: CSS Changes: modifications aren't shared for rules that match multiple elements
+ https://bugs.webkit.org/show_bug.cgi?id=195264
+ <rdar://problem/48550023>
+
+ Reviewed by Devin Rousso.
+
+ This patch fixes several cases when the diff was incorrect.
+
+ 1. Perform diff based on CSSProperty content (name, value, and enabled property) instead
+ of strict equality of CSSProperty instances.
+
+ 2. Copy all initial CSSProperty instances of CSSStyleDeclaration on 1st edit.
+ This removes the need to update `properties` on every single edit.
+
+ 3. Do full diff to display modified property markers (green background) in Rules panel.
+ This fixes a few cases when the markers were inaccurate. E.g. a newly added property
+ matches removed property - no need to show the green background.
+
+ * UserInterface/Base/Utilities.js:
+ (Array.diffArrays):
+ Allow repeating items in the arrays.
+
+ * UserInterface/Controllers/CSSManager.js:
+ (WI.CSSManager.prototype.getModifiedStyle):
+ (WI.CSSManager.prototype.removeModifiedStyle):
+ * UserInterface/Models/CSSProperty.js:
+ (WI.CSSProperty):
+ (WI.CSSProperty.prototype.get modified):
+ (WI.CSSProperty.prototype.set modified):
+ (WI.CSSProperty.prototype.equals):
+ (WI.CSSProperty.prototype.clone):
+ (WI.CSSProperty.prototype._updateOwnerStyleText):
+ (WI.CSSProperty.prototype._markModified):
+ * UserInterface/Models/CSSStyleDeclaration.js:
+ (WI.CSSStyleDeclaration.prototype.markModified):
+ (WI.CSSStyleDeclaration.prototype.updatePropertiesModifiedState):
+ * UserInterface/Views/ChangesDetailsSidebarPanel.js:
+ (WI.ChangesDetailsSidebarPanel.prototype._createRuleElement):
+ * UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js:
+ (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.layout):
+ * UserInterface/Views/SpreadsheetStyleProperty.js:
+
2019-05-31 Devin Rousso <[email protected]>
Web Inspector: Timelines: CPU: gray (?) and (x) should be white
Modified: trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js (245990 => 245991)
--- trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js 2019-05-31 22:53:40 UTC (rev 245991)
@@ -522,62 +522,86 @@
Object.defineProperty(Array, "diffArrays",
{
- value(initialArray, currentArray, onEach)
+ value(initialArray, currentArray, onEach, comparator)
{
- let initialSet = new Set(initialArray);
- let currentSet = new Set(currentArray);
- let indexInitial = 0;
- let indexCurrent = 0;
- let deltaInitial = 0;
- let deltaCurrent = 0;
+ "use strict";
- let i = 0;
- while (true) {
- if (indexInitial >= initialArray.length || indexCurrent >= currentArray.length)
- break;
+ function defaultComparator(initial, current) {
+ return initial === current;
+ }
+ comparator = comparator || defaultComparator;
- let initial = initialArray[indexInitial];
- let current = currentArray[indexCurrent];
+ // Find the shortest prefix of matching items in both arrays.
+ //
+ // initialArray = ["a", "b", "b", "c"]
+ // currentArray = ["c", "b", "b", "a"]
+ // findShortestEdit() // [1, 1]
+ //
+ function findShortestEdit() {
+ let deletionCount = initialArray.length;
+ let additionCount = currentArray.length;
+ let editCount = deletionCount + additionCount;
+ for (let i = 0; i < initialArray.length; ++i) {
+ if (i > editCount) {
+ // Break since any possible edits at this point are going to be longer than the one already found.
+ break;
+ }
- if (initial === current)
- onEach(current, 0);
- else if (currentSet.has(initial)) {
- if (initialSet.has(current)) {
- // Moved.
- onEach(current, 0);
- } else {
- // Added.
- onEach(current, 1);
- --i;
- ++deltaCurrent;
+ for (let j = 0; j < currentArray.length; ++j) {
+ let newEditCount = i + j;
+ if (newEditCount > editCount) {
+ // Break since any possible edits at this point are going to be longer than the one already found.
+ break;
+ }
+
+ if (comparator(initialArray[i], currentArray[j])) {
+ // A candidate for the shortest edit found.
+ if (newEditCount < editCount) {
+ editCount = newEditCount;
+ deletionCount = i;
+ additionCount = j;
+ }
+ break;
+ }
}
- } else {
- // Removed.
- onEach(initial, -1);
- if (!initialSet.has(current)) {
- // Added.
- onEach(current, 1);
- } else {
- --i;
- ++deltaInitial;
- }
}
+ return [deletionCount, additionCount];
+ }
- ++i;
- indexInitial = i + deltaInitial;
- indexCurrent = i + deltaCurrent;
+ function commonPrefixLength(listA, listB) {
+ let shorterListLength = Math.min(listA.length, listB.length);
+ let i = 0;
+ while (i < shorterListLength) {
+ if (!comparator(listA[i], listB[i]))
+ break;
+ ++i;
+ }
+ return i;
}
- for (let i = indexInitial; i < initialArray.length; ++i) {
- // Removed.
- onEach(initialArray[i], -1);
+ function fireOnEach(count, diffAction, array) {
+ for (let i = 0; i < count; ++i)
+ onEach(array[i], diffAction);
}
- for (let i = indexCurrent; i < currentArray.length; ++i) {
- // Added.
- onEach(currentArray[i], 1);
+ while (initialArray.length || currentArray.length) {
+ // Remove common prefix.
+ let prefixLength = commonPrefixLength(initialArray, currentArray);
+ if (prefixLength) {
+ fireOnEach(prefixLength, 0, currentArray);
+ initialArray = initialArray.slice(prefixLength);
+ currentArray = currentArray.slice(prefixLength);
+ }
+
+ if (!initialArray.length && !currentArray.length)
+ break;
+
+ let [deletionCount, additionCount] = findShortestEdit();
+ fireOnEach(deletionCount, -1, initialArray);
+ fireOnEach(additionCount, 1, currentArray);
+ initialArray = initialArray.slice(deletionCount);
+ currentArray = currentArray.slice(additionCount);
}
-
}
});
Modified: trunk/Source/WebInspectorUI/UserInterface/Controllers/CSSManager.js (245990 => 245991)
--- trunk/Source/WebInspectorUI/UserInterface/Controllers/CSSManager.js 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/UserInterface/Controllers/CSSManager.js 2019-05-31 22:53:40 UTC (rev 245991)
@@ -431,6 +431,16 @@
this._modifiedStyles.set(style.stringId, style);
}
+ getModifiedStyle(style)
+ {
+ return this._modifiedStyles.get(style.stringId);
+ }
+
+ removeModifiedStyle(style)
+ {
+ this._modifiedStyles.delete(style.stringId);
+ }
+
// Protected
mediaQueryResultChanged()
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js (245990 => 245991)
--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js 2019-05-31 22:53:40 UTC (rev 245991)
@@ -33,6 +33,7 @@
this._index = index;
this._overridingProperty = null;
this._initialState = null;
+ this._modified = false;
this.update(text, name, value, priority, enabled, overridden, implicit, anonymous, valid, styleSheetTextRange, true);
}
@@ -184,9 +185,18 @@
get modified()
{
- return !!this._initialState;
+ return this._modified;
}
+ set modified(value)
+ {
+ if (this._modified === value)
+ return;
+
+ this._modified = value;
+ this.dispatchEventToListeners(WI.CSSProperty.Event.ModifiedChanged);
+ }
+
get name()
{
return this._name;
@@ -377,6 +387,37 @@
return this._hasOtherVendorNameOrKeyword;
}
+ equals(property)
+ {
+ if (property === this)
+ return true;
+
+ if (!property)
+ return false;
+
+ return this._name === property.name && this._rawValue === property.rawValue && this._enabled === property.enabled;
+ }
+
+ clone()
+ {
+ let cssProperty = new WI.CSSProperty(
+ this._index,
+ this._text,
+ this._name,
+ this._rawValue,
+ this._priority,
+ this._enabled,
+ this._overridden,
+ this._implicit,
+ this._anonymous,
+ this._valid,
+ this._styleSheetTextRange);
+
+ cssProperty.ownerStyle = this._ownerStyle;
+
+ return cssProperty;
+ }
+
// Private
_updateStyleText(forceRemove = false)
@@ -393,8 +434,6 @@
_updateOwnerStyleText(oldText, newText, forceRemove = false)
{
- console.assert(this.modified, "CSSProperty was modified without saving initial state.");
-
if (oldText === newText) {
if (forceRemove) {
const lineDelta = 0;
@@ -437,6 +476,7 @@
let propertyWasRemoved = !newText;
this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, propertyWasRemoved);
+ this._ownerStyle.updatePropertiesModifiedState();
}
_prependSemicolonIfNeeded()
@@ -456,30 +496,13 @@
_markModified()
{
- if (this.modified)
- return;
-
- this._initialState = new WI.CSSProperty(
- this._index,
- this._text,
- this._name,
- this._rawValue,
- this._priority,
- this._enabled,
- this._overridden,
- this._implicit,
- this._anonymous,
- this._valid,
- this._styleSheetTextRange);
-
- if (this._ownerStyle) {
+ if (this._ownerStyle)
this._ownerStyle.markModified();
- this._initialState.ownerStyle = this._ownerStyle.initialState;
- }
}
};
WI.CSSProperty.Event = {
Changed: "css-property-changed",
+ ModifiedChanged: "css-property-modified-changed",
OverriddenStatusChanged: "css-property-overridden-status-changed"
};
Modified: trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js (245990 => 245991)
--- trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js 2019-05-31 22:53:40 UTC (rev 245991)
@@ -365,9 +365,11 @@
markModified()
{
- let properties = this._initialState ? this._initialState.properties : this._properties;
+ if (!this._initialState) {
+ let visibleProperties = this.visibleProperties.map((property) => {
+ return property.clone();
+ });
- if (!this._initialState) {
this._initialState = new WI.CSSStyleDeclaration(
this._nodeStyles,
this._ownerStyleSheet,
@@ -376,12 +378,10 @@
this._node,
this._inherited,
this._text,
- [], // Passing CSS properties here would change their ownerStyle.
+ visibleProperties,
this._styleSheetTextRange);
}
- this._initialState.properties = properties.map((property) => { return property.initialState || property });
-
WI.cssManager.addModifiedStyle(this);
}
@@ -417,6 +417,36 @@
this._visibleProperties = null;
}
+ updatePropertiesModifiedState()
+ {
+ if (!this._initialState)
+ return;
+
+ if (this._type === WI.CSSStyleDeclaration.Type.Computed)
+ return;
+
+ let initialCSSProperties = this._initialState.visibleProperties;
+ let cssProperties = this.visibleProperties;
+
+ let hasModified = false;
+
+ function onEach(cssProperty, action) {
+ if (action !== 0)
+ hasModified = true;
+
+ cssProperty.modified = action ="" 1;
+ }
+
+ function comparator(a, b) {
+ return a.equals(b);
+ }
+
+ Array.diffArrays(initialCSSProperties, cssProperties, onEach, comparator);
+
+ if (!hasModified)
+ WI.cssManager.removeModifiedStyle(this);
+ }
+
// Protected
get nodeStyles()
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ChangesDetailsSidebarPanel.js (245990 => 245991)
--- trunk/Source/WebInspectorUI/UserInterface/Views/ChangesDetailsSidebarPanel.js 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ChangesDetailsSidebarPanel.js 2019-05-31 22:53:40 UTC (rev 245991)
@@ -149,28 +149,28 @@
selectorLineElement.append(" {\n");
- let appendProperty = (cssProperty, className) => {
+ function onEach(cssProperty, action) {
+ let className = "";
+ if (action ="" 1)
+ className = "added";
+ else if (action ="" -1)
+ className = "removed";
+ else
+ className = "unchanged";
+
let propertyLineElement = ruleElement.appendChild(document.createElement("div"));
propertyLineElement.classList.add("css-property-line", className);
- let stylePropertyView = new WI.SpreadsheetStyleProperty(null, cssProperty, {readOnly: true});
+
+ const delegate = null;
+ let stylePropertyView = new WI.SpreadsheetStyleProperty(delegate, cssProperty, {readOnly: true});
propertyLineElement.append(WI.indentString(), stylePropertyView.element, "\n");
- };
+ }
- let initialCSSProperties = style.initialState.visibleProperties;
- let cssProperties = style.visibleProperties;
+ function comparator(a, b) {
+ return a.equals(b);
+ }
- Array.diffArrays(initialCSSProperties, cssProperties, (cssProperty, action) => {
- if (action ="" 0) {
- if (cssProperty.modified) {
- appendProperty(cssProperty.initialState, "removed");
- appendProperty(cssProperty, "added");
- } else
- appendProperty(cssProperty, "unchanged");
- } else if (action ="" 1)
- appendProperty(cssProperty, "added");
- else if (action ="" -1)
- appendProperty(cssProperty, "removed");
- });
+ Array.diffArrays(style.initialState.visibleProperties, style.visibleProperties, onEach, comparator);
let closeBraceElement = document.createElement("span");
closeBraceElement.className = "close-brace";
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js (245990 => 245991)
--- trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js 2019-05-31 22:53:40 UTC (rev 245991)
@@ -82,6 +82,9 @@
this.element.removeChildren();
+ if (this._style)
+ this._style.updatePropertiesModifiedState();
+
let properties = this.propertiesToRender;
this.element.classList.toggle("no-properties", !properties.length);
Modified: trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js (245990 => 245991)
--- trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js 2019-05-31 22:30:55 UTC (rev 245990)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js 2019-05-31 22:53:40 UTC (rev 245991)
@@ -53,6 +53,7 @@
if (!this._readOnly) {
this._element.tabIndex = -1;
+ property.addEventListener(WI.CSSProperty.Event.ModifiedChanged, this.updateStatus, this);
this._element.addEventListener("blur", (event) => {
// Keep selection after tabbing out of Web Inspector window and back.
