Summary: | Web Inspector: Styles: shift-clicking a color-swatch to change formats starts editing the color | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Devin Rousso <hi> | ||||||
Component: | Web Inspector | Assignee: | Nikita Vasilyev <nvasilyev> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, hi, inspector-bugzilla-changes, nvasilyev, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
URL: | https://devinrousso.com/demo/WebKit/test.html | ||||||||
Attachments: |
|
Description
Devin Rousso
2018-12-17 16:06:29 PST
Views/SpreadsheetStyleProperty.js:663:23: CONSOLE ERROR Modified property was unlocked (color) Models/CSSProperty.js:372:23: CONSOLE ERROR _styleSheetTextRange data is invalid. Created attachment 357750 [details]
Patch
Created attachment 357752 [details]
[Animated GIF] With patch applied
Comment on attachment 357750 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357750&action=review rs=me, thanks for the quick fix! > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:484 > + if (this._swatchActive || event.shiftKey) Is there any reason to not prevent editing when clicking on the swatch? Is this needed to make sure multiple-selection works with ⌘-click/⇧-click? If that's the case, should this check instead just be that we aren't in "multiple-selection mode" or that "multiple-selection isn't active"? (In reply to Devin Rousso from comment #5) > Comment on attachment 357750 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=357750&action=review > > rs=me, thanks for the quick fix! > > > Source/WebInspectorUI/UserInterface/Views/SpreadsheetStyleProperty.js:484 > > + if (this._swatchActive || event.shiftKey) > > Is there any reason to not prevent editing when clicking on the swatch? Is > this needed to make sure multiple-selection works with ⌘-click/⇧-click? If > that's the case, should this check instead just be that we aren't in > "multiple-selection mode" or that "multiple-selection isn't active"? Ideally, we should always prevent editing when clicking on the swatch. If I replace it with: swatch.element.addEventListener("click", (event) => { event.stop(); }); Inline swatches stop working after the first click! This happens because _swatchElementClicked removes itself: if (this._boundSwatchElementClicked) this._swatchElement.removeEventListener("click", this._boundSwatchElementClicked); and when this event listener gets added back (in didDismissPopover), it ends up *after* the `event.stop()` event listener. I'd like to rewrite InlineSwatch so I don't need to call `event.stop` in SpreadsheetStyleProperty at all. I'm planning to do after the break, not today or tomorrow. Comment on attachment 357750 [details] Patch Clearing flags on attachment: 357750 Committed r239413: <https://trac.webkit.org/changeset/239413> All reviewed patches have been landed. Closing bug. |