Changeset 245991 in webkit
- Timestamp:
- May 31, 2019, 3:53:40 PM (7 years ago)
- Location:
- trunk
- Files:
-
- 11 edited
-
LayoutTests/ChangeLog (modified) (1 diff)
-
LayoutTests/inspector/unit-tests/array-utilities-expected.txt (modified) (1 diff)
-
LayoutTests/inspector/unit-tests/array-utilities.html (modified) (1 diff)
-
Source/WebInspectorUI/ChangeLog (modified) (1 diff)
-
Source/WebInspectorUI/UserInterface/Base/Utilities.js (modified) (1 diff)
-
Source/WebInspectorUI/UserInterface/Controllers/CSSManager.js (modified) (1 diff)
-
Source/WebInspectorUI/UserInterface/Models/CSSProperty.js (modified) (5 diffs)
-
Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js (modified) (3 diffs)
-
Source/WebInspectorUI/UserInterface/Views/ChangesDetailsSidebarPanel.js (modified) (1 diff)
-
Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js (modified) (1 diff)
-
Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js (modified) (1 diff)
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r245990 r245991 1 2019-05-31 Nikita Vasilyev <nvasilyev@apple.com> 2 3 Web Inspector: CSS Changes: modifications aren't shared for rules that match multiple elements 4 https://bugs.webkit.org/show_bug.cgi?id=195264 5 <rdar://problem/48550023> 6 7 Reviewed by Devin Rousso. 8 9 Test arrays with repeating items for Array.diffArrays. 10 11 * inspector/unit-tests/array-utilities-expected.txt: 12 * inspector/unit-tests/array-utilities.html: 13 1 14 2019-05-31 Ryosuke Niwa <rniwa@webkit.org> 2 15 -
trunk/LayoutTests/inspector/unit-tests/array-utilities-expected.txt
r243242 r245991 82 82 ["b","a"], ["a","c"] => [["b",-1],["a",0],["c",1]] 83 83 ["b","a"], ["a","c"] => [["b",-1],["a",0],["c",1]] 84 ["b","a"], ["a","b"] => [["a", 0],["b",0]]84 ["b","a"], ["a","b"] => [["a",1],["b",0],["a",-1]] 85 85 ["a","b","c"], ["a","d","c"] => [["a",0],["b",-1],["d",1],["c",0]] 86 ["a","b","c"], ["c","b","a"] => [["c",1],["b",1],["a",0],["b",-1],["c",-1]] 87 88 Repeating items: 89 ["a"], ["a","a"] => [["a",0],["a",1]] 90 ["a","a"], ["a"] => [["a",0],["a",-1]] 91 ["a","a"], ["a","a"] => [["a",0],["a",0]] 92 ["b","a","b"], ["a","b","a"] => [["a",1],["b",0],["a",0],["b",-1]] 93 ["a","b","b","c"], ["c","b","b","b","a"] => [["a",-1],["c",1],["b",0],["b",0],["c",-1],["b",1],["a",1]] 94 ["a","b","b","b","c"], ["c","b","b","a"] => [["a",-1],["c",1],["b",0],["b",0],["b",-1],["c",-1],["a",1]] 95 ["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]] 86 96 87 97 -- Running test case: Array.prototype.lastValue -
trunk/LayoutTests/inspector/unit-tests/array-utilities.html
r243242 r245991 176 176 diff(["b", "a"], ["a", "b"]); 177 177 diff(["a", "b", "c"], ["a", "d", "c"]); 178 diff(["a", "b", "c"], ["c", "b", "a"]); 179 180 InspectorTest.log("\nRepeating items:"); 181 diff(["a"], ["a", "a"]); 182 diff(["a", "a"], ["a"]); 183 diff(["a", "a"], ["a", "a"]); 184 diff(["b", "a", "b"], ["a", "b", "a"]); 185 diff(["a", "b", "b", "c"], ["c", "b", "b", "b", "a"]); 186 diff(["a", "b", "b", "b", "c"], ["c", "b", "b", "a"]); 187 diff(["a", "a", "b", "b", "c", "c"], ["b", "b", "c", "c", "a", "a"]); 178 188 179 189 return true; -
trunk/Source/WebInspectorUI/ChangeLog
r245976 r245991 1 2019-05-31 Nikita Vasilyev <nvasilyev@apple.com> 2 3 Web Inspector: CSS Changes: modifications aren't shared for rules that match multiple elements 4 https://bugs.webkit.org/show_bug.cgi?id=195264 5 <rdar://problem/48550023> 6 7 Reviewed by Devin Rousso. 8 9 This patch fixes several cases when the diff was incorrect. 10 11 1. Perform diff based on CSSProperty content (name, value, and enabled property) instead 12 of strict equality of CSSProperty instances. 13 14 2. Copy all initial CSSProperty instances of CSSStyleDeclaration on 1st edit. 15 This removes the need to update `properties` on every single edit. 16 17 3. Do full diff to display modified property markers (green background) in Rules panel. 18 This fixes a few cases when the markers were inaccurate. E.g. a newly added property 19 matches removed property - no need to show the green background. 20 21 * UserInterface/Base/Utilities.js: 22 (Array.diffArrays): 23 Allow repeating items in the arrays. 24 25 * UserInterface/Controllers/CSSManager.js: 26 (WI.CSSManager.prototype.getModifiedStyle): 27 (WI.CSSManager.prototype.removeModifiedStyle): 28 * UserInterface/Models/CSSProperty.js: 29 (WI.CSSProperty): 30 (WI.CSSProperty.prototype.get modified): 31 (WI.CSSProperty.prototype.set modified): 32 (WI.CSSProperty.prototype.equals): 33 (WI.CSSProperty.prototype.clone): 34 (WI.CSSProperty.prototype._updateOwnerStyleText): 35 (WI.CSSProperty.prototype._markModified): 36 * UserInterface/Models/CSSStyleDeclaration.js: 37 (WI.CSSStyleDeclaration.prototype.markModified): 38 (WI.CSSStyleDeclaration.prototype.updatePropertiesModifiedState): 39 * UserInterface/Views/ChangesDetailsSidebarPanel.js: 40 (WI.ChangesDetailsSidebarPanel.prototype._createRuleElement): 41 * UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js: 42 (WI.SpreadsheetCSSStyleDeclarationEditor.prototype.layout): 43 * UserInterface/Views/SpreadsheetStyleProperty.js: 44 1 45 2019-05-31 Devin Rousso <drousso@apple.com> 2 46 -
trunk/Source/WebInspectorUI/UserInterface/Base/Utilities.js
r245914 r245991 523 523 Object.defineProperty(Array, "diffArrays", 524 524 { 525 value(initialArray, currentArray, onEach) 526 { 527 let initialSet = new Set(initialArray); 528 let currentSet = new Set(currentArray); 529 let indexInitial = 0; 530 let indexCurrent = 0; 531 let deltaInitial = 0; 532 let deltaCurrent = 0; 533 534 let i = 0; 535 while (true) { 536 if (indexInitial >= initialArray.length || indexCurrent >= currentArray.length) 537 break; 538 539 let initial = initialArray[indexInitial]; 540 let current = currentArray[indexCurrent]; 541 542 if (initial === current) 543 onEach(current, 0); 544 else if (currentSet.has(initial)) { 545 if (initialSet.has(current)) { 546 // Moved. 547 onEach(current, 0); 548 } else { 549 // Added. 550 onEach(current, 1); 551 --i; 552 ++deltaCurrent; 525 value(initialArray, currentArray, onEach, comparator) 526 { 527 "use strict"; 528 529 function defaultComparator(initial, current) { 530 return initial === current; 531 } 532 comparator = comparator || defaultComparator; 533 534 // Find the shortest prefix of matching items in both arrays. 535 // 536 // initialArray = ["a", "b", "b", "c"] 537 // currentArray = ["c", "b", "b", "a"] 538 // findShortestEdit() // [1, 1] 539 // 540 function findShortestEdit() { 541 let deletionCount = initialArray.length; 542 let additionCount = currentArray.length; 543 let editCount = deletionCount + additionCount; 544 for (let i = 0; i < initialArray.length; ++i) { 545 if (i > editCount) { 546 // Break since any possible edits at this point are going to be longer than the one already found. 547 break; 553 548 } 554 } else { 555 // Removed. 556 onEach(initial, -1); 557 if (!initialSet.has(current)) { 558 // Added. 559 onEach(current, 1); 560 } else { 561 --i; 562 ++deltaInitial; 549 550 for (let j = 0; j < currentArray.length; ++j) { 551 let newEditCount = i + j; 552 if (newEditCount > editCount) { 553 // Break since any possible edits at this point are going to be longer than the one already found. 554 break; 555 } 556 557 if (comparator(initialArray[i], currentArray[j])) { 558 // A candidate for the shortest edit found. 559 if (newEditCount < editCount) { 560 editCount = newEditCount; 561 deletionCount = i; 562 additionCount = j; 563 } 564 break; 565 } 563 566 } 564 567 } 565 566 ++i; 567 indexInitial = i + deltaInitial; 568 indexCurrent = i + deltaCurrent; 569 } 570 571 for (let i = indexInitial; i < initialArray.length; ++i) { 572 // Removed. 573 onEach(initialArray[i], -1); 574 } 575 576 for (let i = indexCurrent; i < currentArray.length; ++i) { 577 // Added. 578 onEach(currentArray[i], 1); 579 } 580 568 return [deletionCount, additionCount]; 569 } 570 571 function commonPrefixLength(listA, listB) { 572 let shorterListLength = Math.min(listA.length, listB.length); 573 let i = 0; 574 while (i < shorterListLength) { 575 if (!comparator(listA[i], listB[i])) 576 break; 577 ++i; 578 } 579 return i; 580 } 581 582 function fireOnEach(count, diffAction, array) { 583 for (let i = 0; i < count; ++i) 584 onEach(array[i], diffAction); 585 } 586 587 while (initialArray.length || currentArray.length) { 588 // Remove common prefix. 589 let prefixLength = commonPrefixLength(initialArray, currentArray); 590 if (prefixLength) { 591 fireOnEach(prefixLength, 0, currentArray); 592 initialArray = initialArray.slice(prefixLength); 593 currentArray = currentArray.slice(prefixLength); 594 } 595 596 if (!initialArray.length && !currentArray.length) 597 break; 598 599 let [deletionCount, additionCount] = findShortestEdit(); 600 fireOnEach(deletionCount, -1, initialArray); 601 fireOnEach(additionCount, 1, currentArray); 602 initialArray = initialArray.slice(deletionCount); 603 currentArray = currentArray.slice(additionCount); 604 } 581 605 } 582 606 }); -
trunk/Source/WebInspectorUI/UserInterface/Controllers/CSSManager.js
r243208 r245991 432 432 } 433 433 434 getModifiedStyle(style) 435 { 436 return this._modifiedStyles.get(style.stringId); 437 } 438 439 removeModifiedStyle(style) 440 { 441 this._modifiedStyles.delete(style.stringId); 442 } 443 434 444 // Protected 435 445 -
trunk/Source/WebInspectorUI/UserInterface/Models/CSSProperty.js
r242622 r245991 34 34 this._overridingProperty = null; 35 35 this._initialState = null; 36 this._modified = false; 36 37 37 38 this.update(text, name, value, priority, enabled, overridden, implicit, anonymous, valid, styleSheetTextRange, true); … … 185 186 get modified() 186 187 { 187 return !!this._initialState; 188 return this._modified; 189 } 190 191 set modified(value) 192 { 193 if (this._modified === value) 194 return; 195 196 this._modified = value; 197 this.dispatchEventToListeners(WI.CSSProperty.Event.ModifiedChanged); 188 198 } 189 199 … … 378 388 } 379 389 380 // Private 381 382 _updateStyleText(forceRemove = false) 383 { 384 let text = ""; 385 386 if (this._name && this._rawValue) 387 text = this._name + ": " + this._rawValue + ";"; 388 389 let oldText = this._text; 390 this._text = text; 391 this._updateOwnerStyleText(oldText, this._text, forceRemove); 392 } 393 394 _updateOwnerStyleText(oldText, newText, forceRemove = false) 395 { 396 console.assert(this.modified, "CSSProperty was modified without saving initial state."); 397 398 if (oldText === newText) { 399 if (forceRemove) { 400 const lineDelta = 0; 401 const columnDelta = 0; 402 this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, forceRemove); 403 } 404 return; 405 } 406 407 console.assert(this._ownerStyle); 408 if (!this._ownerStyle) 409 return; 410 411 this._prependSemicolonIfNeeded(); 412 413 let styleText = this._ownerStyle.text || ""; 414 415 // _styleSheetTextRange is the position of the property within the stylesheet. 416 // range is the position of the property within the rule. 417 let range = this._styleSheetTextRange.relativeTo(this._ownerStyle.styleSheetTextRange.startLine, this._ownerStyle.styleSheetTextRange.startColumn); 418 419 // Append a line break to count the last line of styleText towards endOffset. 420 range.resolveOffsets(styleText + "\n"); 421 422 console.assert(oldText === styleText.slice(range.startOffset, range.endOffset), "_styleSheetTextRange data is invalid."); 423 424 if (WI.settings.enableStyleEditingDebugMode.value) { 425 let prefix = styleText.slice(0, range.startOffset); 426 let postfix = styleText.slice(range.endOffset); 427 console.info(`${prefix}%c${oldText}%c${newText}%c${postfix}`, `background: hsl(356, 100%, 90%); color: black`, `background: hsl(100, 100%, 91%); color: black`, `background: transparent`); 428 } 429 430 let newStyleText = styleText.slice(0, range.startOffset) + newText + styleText.slice(range.endOffset); 431 432 let lineDelta = newText.lineCount - oldText.lineCount; 433 let columnDelta = newText.lastLine.length - oldText.lastLine.length; 434 this._styleSheetTextRange = this._styleSheetTextRange.cloneAndModify(0, 0, lineDelta, columnDelta); 435 436 this._ownerStyle.text = newStyleText; 437 438 let propertyWasRemoved = !newText; 439 this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, propertyWasRemoved); 440 } 441 442 _prependSemicolonIfNeeded() 443 { 444 for (let i = this.index - 1; i >= 0; --i) { 445 let property = this._ownerStyle.properties[i]; 446 if (!property.enabled) 447 continue; 448 449 let match = property.text.match(/[^;\s](\s*)$/); 450 if (match) 451 property.text = property.text.trimRight() + ";" + match[1]; 452 453 break; 454 } 455 } 456 457 _markModified() 458 { 459 if (this.modified) 460 return; 461 462 this._initialState = new WI.CSSProperty( 390 equals(property) 391 { 392 if (property === this) 393 return true; 394 395 if (!property) 396 return false; 397 398 return this._name === property.name && this._rawValue === property.rawValue && this._enabled === property.enabled; 399 } 400 401 clone() 402 { 403 let cssProperty = new WI.CSSProperty( 463 404 this._index, 464 405 this._text, … … 473 414 this._styleSheetTextRange); 474 415 475 if (this._ownerStyle) { 416 cssProperty.ownerStyle = this._ownerStyle; 417 418 return cssProperty; 419 } 420 421 // Private 422 423 _updateStyleText(forceRemove = false) 424 { 425 let text = ""; 426 427 if (this._name && this._rawValue) 428 text = this._name + ": " + this._rawValue + ";"; 429 430 let oldText = this._text; 431 this._text = text; 432 this._updateOwnerStyleText(oldText, this._text, forceRemove); 433 } 434 435 _updateOwnerStyleText(oldText, newText, forceRemove = false) 436 { 437 if (oldText === newText) { 438 if (forceRemove) { 439 const lineDelta = 0; 440 const columnDelta = 0; 441 this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, forceRemove); 442 } 443 return; 444 } 445 446 console.assert(this._ownerStyle); 447 if (!this._ownerStyle) 448 return; 449 450 this._prependSemicolonIfNeeded(); 451 452 let styleText = this._ownerStyle.text || ""; 453 454 // _styleSheetTextRange is the position of the property within the stylesheet. 455 // range is the position of the property within the rule. 456 let range = this._styleSheetTextRange.relativeTo(this._ownerStyle.styleSheetTextRange.startLine, this._ownerStyle.styleSheetTextRange.startColumn); 457 458 // Append a line break to count the last line of styleText towards endOffset. 459 range.resolveOffsets(styleText + "\n"); 460 461 console.assert(oldText === styleText.slice(range.startOffset, range.endOffset), "_styleSheetTextRange data is invalid."); 462 463 if (WI.settings.enableStyleEditingDebugMode.value) { 464 let prefix = styleText.slice(0, range.startOffset); 465 let postfix = styleText.slice(range.endOffset); 466 console.info(`${prefix}%c${oldText}%c${newText}%c${postfix}`, `background: hsl(356, 100%, 90%); color: black`, `background: hsl(100, 100%, 91%); color: black`, `background: transparent`); 467 } 468 469 let newStyleText = styleText.slice(0, range.startOffset) + newText + styleText.slice(range.endOffset); 470 471 let lineDelta = newText.lineCount - oldText.lineCount; 472 let columnDelta = newText.lastLine.length - oldText.lastLine.length; 473 this._styleSheetTextRange = this._styleSheetTextRange.cloneAndModify(0, 0, lineDelta, columnDelta); 474 475 this._ownerStyle.text = newStyleText; 476 477 let propertyWasRemoved = !newText; 478 this._ownerStyle.shiftPropertiesAfter(this, lineDelta, columnDelta, propertyWasRemoved); 479 this._ownerStyle.updatePropertiesModifiedState(); 480 } 481 482 _prependSemicolonIfNeeded() 483 { 484 for (let i = this.index - 1; i >= 0; --i) { 485 let property = this._ownerStyle.properties[i]; 486 if (!property.enabled) 487 continue; 488 489 let match = property.text.match(/[^;\s](\s*)$/); 490 if (match) 491 property.text = property.text.trimRight() + ";" + match[1]; 492 493 break; 494 } 495 } 496 497 _markModified() 498 { 499 if (this._ownerStyle) 476 500 this._ownerStyle.markModified(); 477 this._initialState.ownerStyle = this._ownerStyle.initialState;478 }479 501 } 480 502 }; … … 482 504 WI.CSSProperty.Event = { 483 505 Changed: "css-property-changed", 506 ModifiedChanged: "css-property-modified-changed", 484 507 OverriddenStatusChanged: "css-property-overridden-status-changed" 485 508 }; -
trunk/Source/WebInspectorUI/UserInterface/Models/CSSStyleDeclaration.js
r243256 r245991 366 366 markModified() 367 367 { 368 let properties = this._initialState ? this._initialState.properties : this._properties;369 370 368 if (!this._initialState) { 369 let visibleProperties = this.visibleProperties.map((property) => { 370 return property.clone(); 371 }); 372 371 373 this._initialState = new WI.CSSStyleDeclaration( 372 374 this._nodeStyles, … … 377 379 this._inherited, 378 380 this._text, 379 [], // Passing CSS properties here would change their ownerStyle.381 visibleProperties, 380 382 this._styleSheetTextRange); 381 383 } 382 383 this._initialState.properties = properties.map((property) => { return property.initialState || property });384 384 385 385 WI.cssManager.addModifiedStyle(this); … … 418 418 } 419 419 420 updatePropertiesModifiedState() 421 { 422 if (!this._initialState) 423 return; 424 425 if (this._type === WI.CSSStyleDeclaration.Type.Computed) 426 return; 427 428 let initialCSSProperties = this._initialState.visibleProperties; 429 let cssProperties = this.visibleProperties; 430 431 let hasModified = false; 432 433 function onEach(cssProperty, action) { 434 if (action !== 0) 435 hasModified = true; 436 437 cssProperty.modified = action === 1; 438 } 439 440 function comparator(a, b) { 441 return a.equals(b); 442 } 443 444 Array.diffArrays(initialCSSProperties, cssProperties, onEach, comparator); 445 446 if (!hasModified) 447 WI.cssManager.removeModifiedStyle(this); 448 } 449 420 450 // Protected 421 451 -
trunk/Source/WebInspectorUI/UserInterface/Views/ChangesDetailsSidebarPanel.js
r243038 r245991 150 150 selectorLineElement.append(" {\n"); 151 151 152 let appendProperty = (cssProperty, className) => { 152 function onEach(cssProperty, action) { 153 let className = ""; 154 if (action === 1) 155 className = "added"; 156 else if (action === -1) 157 className = "removed"; 158 else 159 className = "unchanged"; 160 153 161 let propertyLineElement = ruleElement.appendChild(document.createElement("div")); 154 162 propertyLineElement.classList.add("css-property-line", className); 155 let stylePropertyView = new WI.SpreadsheetStyleProperty(null, cssProperty, {readOnly: true}); 163 164 const delegate = null; 165 let stylePropertyView = new WI.SpreadsheetStyleProperty(delegate, cssProperty, {readOnly: true}); 156 166 propertyLineElement.append(WI.indentString(), stylePropertyView.element, "\n"); 157 }; 158 159 let initialCSSProperties = style.initialState.visibleProperties; 160 let cssProperties = style.visibleProperties; 161 162 Array.diffArrays(initialCSSProperties, cssProperties, (cssProperty, action) => { 163 if (action === 0) { 164 if (cssProperty.modified) { 165 appendProperty(cssProperty.initialState, "removed"); 166 appendProperty(cssProperty, "added"); 167 } else 168 appendProperty(cssProperty, "unchanged"); 169 } else if (action === 1) 170 appendProperty(cssProperty, "added"); 171 else if (action === -1) 172 appendProperty(cssProperty, "removed"); 173 }); 167 } 168 169 function comparator(a, b) { 170 return a.equals(b); 171 } 172 173 Array.diffArrays(style.initialState.visibleProperties, style.visibleProperties, onEach, comparator); 174 174 175 175 let closeBraceElement = document.createElement("span"); -
trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetCSSStyleDeclarationEditor.js
r242622 r245991 82 82 83 83 this.element.removeChildren(); 84 85 if (this._style) 86 this._style.updatePropertiesModifiedState(); 84 87 85 88 let properties = this.propertiesToRender; -
trunk/Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js
r242731 r245991 54 54 if (!this._readOnly) { 55 55 this._element.tabIndex = -1; 56 property.addEventListener(WI.CSSProperty.Event.ModifiedChanged, this.updateStatus, this); 56 57 57 58 this._element.addEventListener("blur", (event) => {
Note:
See TracChangeset
for help on using the changeset viewer.