[webkit-reviews] review granted: [Bug 177346] Web Inspector: Add autocompletion suggestions for CSS attr based on corresponding element's dataset : [Attachment 321522] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<webkit at devinrousso.com>'s request for review:
Bug 177346: Web Inspector: Add autocompletion suggestions for CSS attr based on
corresponding element's dataset
https://bugs.webkit.org/show_bug.cgi?id=177346

Attachment 321522: Patch

https://bugs.webkit.org/attachment.cgi?id=321522&action=review




--- Comment #3 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 321522
  --> https://bugs.webkit.org/attachment.cgi?id=321522
Patch

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

We could use this as an opportunity to write a test for
`DOMNode.prototype.attributes`, which this depends on and which we don't have
any existing tests for! I suggest starting a new:
LayoutTests/inspector/model/dom-node.html, then as we want to test different
parts of DOMNode we just add a small method. A test for attributes can just
have a few different nodes with different attributes and log their attributes +
values.

>
Source/WebInspectorUI/UserInterface/Controllers/CodeMirrorCompletionController.
js:589
> +	       if (this._delegate && typeof
this._delegate.completionControllerCSSFunctionValuesNeeded)
> +		   functionCompletions =
this._delegate.completionControllerCSSFunctionValuesNeeded(this, functionName,
functionCompletions);

This feels a bit hacky, but its short and simple.

>
Source/WebInspectorUI/UserInterface/Views/CSSStyleDeclarationTextEditor.js:402
> +		   if (!attribute.name.startsWith("data-"))
> +		       continue;

attr() can be used with any attribute, not just "data-" prefixed attributes. A
common usage may be `title` or `alt`:

    <style>p::before { color: blue; content: attr(title); }</style>
    <p title="Test">Hello</p>

So I think we should just push all attribute names.

I don't know how this would work with XML namespaced attributes. Maybe we
should ignore those (they may also be unlikely).