Bug 128965

Summary: Web Inspector: editing a color in the Styles sidebar using the color picker only works once for a given color
Product: WebKit Reporter: Antoine Quint <graouts>
Component: Web InspectorAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dpino, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch none

Antoine Quint
Reported 2014-02-18 03:25:12 PST
Steps to reproduce: 1. select any DOM node with a style property set to a color value 2. show the Styles sidebar 3. click on the color swatch for that value 4. click anywhere on the color wheel, notice how the color in the editor is changed to match 5. click anywhere on the color wheel, notice how the color _does not_ change to match in the editor I believe the CodeMirror text marker managed by the WebInspector.TextMarker object is no longer present when we try to update its text from the WebInspector.CodeMirrorColorEditingController.
Attachments
Patch (2.49 KB, patch)
2014-02-18 03:53 PST, Antoine Quint
no flags
Radar WebKit Bug Importer
Comment 1 2014-02-18 03:25:38 PST
Antoine Quint
Comment 2 2014-02-18 03:32:19 PST
The CSSStyleDeclarationTextEditor does not use a CodeMirrorColorEditingController. However, the issue is that in the nested function updateCodeMirror() in CSSStyleDeclarationTextEditor.prototype._colorSwatchClicked() we do not find a valid colorTextMarker past the first update and thus we cannot update the text in the editor.
Antoine Quint
Comment 3 2014-02-18 03:53:19 PST
Timothy Hatcher
Comment 4 2014-02-18 07:25:38 PST
Comment on attachment 224490 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224490&action=review > Source/WebInspectorUI/UserInterface/CSSStyleDeclarationTextEditor.js:649 > + if (WebInspector.TextMarker.textMarkerForCodeMirrorTextMarker(mark).type !== WebInspector.TextMarker.Type.Color) Where does the color type get set? The market on line 674 is just a CodeMirror marker. > Source/WebInspectorUI/UserInterface/CSSStyleDeclarationTextEditor.js:674 > colorTextMarker = this._codeMirror.markText(range.from, range.to); Doesn't this need to set the type using our API to mark the text?
Antoine Quint
Comment 5 2014-02-18 07:39:32 PST
(In reply to comment #4) > (From update of attachment 224490 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=224490&action=review > > > Source/WebInspectorUI/UserInterface/CSSStyleDeclarationTextEditor.js:649 > > + if (WebInspector.TextMarker.textMarkerForCodeMirrorTextMarker(mark).type !== WebInspector.TextMarker.Type.Color) > > Where does the color type get set? The market on line 674 is just a CodeMirror marker. It's set in the CodeMirror extension method .createColorMarkers() which is called from CSSStyleDeclarationTextEditor in ._createColorSwatches(). > > Source/WebInspectorUI/UserInterface/CSSStyleDeclarationTextEditor.js:674 > > colorTextMarker = this._codeMirror.markText(range.from, range.to); > > Doesn't this need to set the type using our API to mark the text? Not anymore, we'll get the type from the WebInspector.TextMarker created for this CodeMirror TextMarker.
Timothy Hatcher
Comment 6 2014-02-18 13:10:46 PST
*** Bug 128768 has been marked as a duplicate of this bug. ***
WebKit Commit Bot
Comment 7 2014-02-18 13:39:38 PST
Comment on attachment 224490 [details] Patch Clearing flags on attachment: 224490 Committed r164312: <http://trac.webkit.org/changeset/164312>
WebKit Commit Bot
Comment 8 2014-02-18 13:39:40 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.