Bug 128965 – Web Inspector: editing a color in the Styles sidebar using the color picker only works once for a given color

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Antoine Quint]

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.

[Radar WebKit Bug Importer]

<rdar://problem/16096027>

[Antoine Quint]

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]

Created attachment 224490 [details]
Patch

[Timothy Hatcher]

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]

(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]

*** Bug 128768 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 224490 [details]
Patch

Clearing flags on attachment: 224490

Committed r164312: <http://trac.webkit.org/changeset/164312>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.