Bug 119686 – Web Inspector: rich editing of CSS gradients

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

[Antoine Quint]

In order to alleviate the complexity of the CSS gradients syntax and make it more visually efficient to edit them, we should offer rich editing of gradients within Web Inspector, for instance in the form of a popover allowing to edit the gradient type, direction and stops.

[Radar WebKit Bug Importer]

<rdar://problem/14712396>

[Antoine Quint]

I think that to do this right, we'll need backend support to do the parsing of the CSS value, which we should be able to do via CSSParser::parseGeneratedImage() and checking if the passed value is of type CSSGradientValue.

[Dean Jackson]

(In reply to comment #2)
> I think that to do this right, we'll need backend support to do the parsing of the CSS value, which we should be able to do via CSSParser::parseGeneratedImage() and checking if the passed value is of type CSSGradientValue.

Note that CSSParser::parse*Gradient take CSSParserValueLists rather than strings so you'll have to treat this like a new stylesheet. And the output is designed to go onto a RenderStyle.

[Antoine Quint]

Another option is to do the parsing in JS, there is existing work in that area, for instance Daniel Glazman's JSCSSP has support for parsing gradients: http://sources.disruptive-innovations.com/jscssp/trunk/src/parser/gradients.js.

[Timothy Hatcher]

I think having our WebKit parser sense. But at the same time, if you use a newer Web Inspector with an older device you will be locked into the old syntax. Having a JS parser lets the Inspector handle new syntaxes that the device might not support but are new and valid.

[Timothy Hatcher]

I bungled that last message.

"I think having our WebKit parser makes some sense."

"Having a JS CSS parser lets…"

[Antoine Quint]

Work on this feature is being done on my own WebKit fork at https://github.com/graouts/webkit/tree/gradients. It's mostly done, save for integration in the CSSStylesDeclarationTextEditor.

[Antoine Quint]

The blocking bugs are:

https://bugs.webkit.org/show_bug.cgi?id=129088
https://bugs.webkit.org/show_bug.cgi?id=129089
https://bugs.webkit.org/show_bug.cgi?id=129094

The good news is that they all have patches out for review, and the patch for this bug is ready to be sent out as soon as the blocking bugs have landed.

[Antoine Quint]

Some followup tasks:

http://webkit.org/b/129095
http://webkit.org/b/129096

[Antoine Quint]

Created attachment 224777 [details]
Patch

[Timothy Hatcher]

Comment on attachment 224777 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=224777&action=review

> Source/WebInspectorUI/UserInterface/CodeMirrorGradientEditingController.js:177
> +        var matches = this._angleInput.value.match(/º/g);

Should unicode escape the symbol.

> Source/WebInspectorUI/UserInterface/CodeMirrorGradientEditingController.js:180
> +            this._angleInput.value = angle + "º";

Ditto.

> Source/WebInspectorUI/UserInterface/CodeMirrorGradientEditingController.js:192
> +                this._angleInput.value = "180º";

Ditto.

> Source/WebInspectorUI/UserInterface/GradientSlider.css:103
> +    border: 1px solid rgba(0,0,0,0.25);

Spaces after the commas.

> Source/WebInspectorUI/UserInterface/GradientSlider.js:357
> +        if (selected)
> +            this._element.classList.add(WebInspector.GradientSliderKnob.SelectedClassName);
> +        else
> +            this._element.classList.remove(WebInspector.GradientSliderKnob.SelectedClassName);

This can be one line: this._element.classList.toggle(WebInspector.GradientSliderKnob.SelectedClassName, selected);

> Source/WebInspectorUI/UserInterface/Images/Checkers.svg:2
> +<svg xmlns="http://www.w3.org/2000/svg" width="6" height="6" fill="rgb(204, 204, 204)"><rect width="3" height="3" /><rect x="3" y="3" width="3" height="3"/></svg>
>  \ No newline at end of file

This should get the license header comment and have newlines.

[Antoine Quint]

Landed r164446 addressing Tim's feedback: <http://trac.webkit.org/changeset/164446>