Web Inspector: Add "unset" to CSS value autocompletion

• Component: Web Inspector
• Status: RESOLVED FIXED

[Diego Pino]

When editing a CSS value, a completion list of suggested list shows up. The list of values depends on the type of property. In the current implementation, the value 'inherit' is only available for inheritable properties.

According to the "CSS Cascading and Inheritance Level 3" recommendation, the value 'inherit' is valid for any CSS property, not only for those which are inheritable.

http://www.w3.org/TR/css3-cascade/
https://developer.mozilla.org/en-US/docs/Web/CSS/inherit
https://developer.mozilla.org/en-US/docs/Web/CSS/inheritance

In addition, the value 'unset' should also be added to the list of suggested values, as it can be applied to any CSS property. The value 'unset' works as 'inherit' for inheritable properties and works as 'initial' for non-inheritable properties.

https://developer.mozilla.org/en-US/docs/Web/CSS/unset

[Radar WebKit Bug Importer]

<rdar://problem/15908963>

[Diego Pino]

Created attachment 222204 [details]
Patch

[Timothy Hatcher]

Comment on attachment 222204 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/CSSKeywordCompletions.js:36
> +    var acceptedKeywords = ["inherit", "initial", "unset"];

According to MDN no browser supports "unset". If that is true, let's not add it. We only show completions for things WebKit actually supports.

[Joseph Pecoraro]

Comment on attachment 222204 [details]
Patch

We discussed this on IRC. I'm going to r-. Although this is technically correct, our existing behavior is smart and would be more useful to users.

Currently we always autocomplete "initial", and we only autocomplete "inherit" on inheritable properties (e.g. color, but not background-image).

That makes sense, because on a non-inheritable property, "inherit" would just mean "initial", the default value.

And, like Tim suggested, we don't support "unset" yet. We don't want to show properties in the frontend that a backend does not yet support. Currently if you typed "color: unset" in Web Inspector, it would get a red strike through. That would be weird if we showed it as an autocomplete suggestion =).

[Joseph Pecoraro]

Comment on attachment 222204 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/CSSKeywordCompletions.js:-49
> -    // FIXME: This doesn't necessarily make sense. Some non-inheritable properties allow "inherit".

You probably wrote this patch because of the FIXME comment. So that FIXME can be removed and replaced with a descriptive comment

  // Only suggest "inherit" on inheritable properties even though it is valid on all properties.

[Diego Pino]

Created attachment 222241 [details]
Test of initial and inherit values on non-inherited property (background-image)

I have attached a page showing how different boxes are rendered setting different values (initial, inherit, etc) for background-image (non-inherited property).

At this point I can conclude the following:

   * When an inherited property is not set, its value is set to the value of its parent.
   * When a non-inherited property is not set, its value is set to its initial value (spec specifies what that value is, http://www.w3.org/TR/CSS2/propidx.html).
   * Setting an inherited property to inherit is the same as setting no value for that property.
   * Setting a non-inherited property to initial is the same as setting no value for that property.
   * Initial and inherit can be applied to either inherited and non-inherited properties.

As Joseph told me on IRC, setting "inherit" to a non-inherited property in the current implementation is valid, because it's not crossed through. The point is that the autocompletion doesn't suggest "inherit" on non-inherited properties. When the user starts typing "in" it will only suggest "initial" for non-inherited properties (and suggests "inherit" and "initial" for inherited properties). The good thing of the current implementation is that it's easy to know what properties are inherited and which ones are not just by looking at the values suggested (if inherit is not suggested it means the property is non-inherited).

With regard to "unset", I agree with removing it.

[Joseph Pecoraro]

I opened <https://bugs.webkit.org/show_bug.cgi?id=127638> about removing the FIXME.

This bug can either be closed, or remain open to add "unset" when WebKit supports unset. Retitling as such.

[Joseph Pecoraro]

Comment on attachment 222204 [details]
Patch

Seems this can be addressed now that "unset" has landed. The patch likely needs an update and I don't know if the semantics for "unset" changed.

[Joseph Pecoraro]

Seems easy enough!

[Joseph Pecoraro]

Created attachment 263245 [details]
[PATCH] Proposed Fix

[BJ Burg]

Comment on attachment 263245 [details]
[PATCH] Proposed Fix

r=me

[WebKit Commit Bot]

Comment on attachment 263245 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 263245

Committed r191187: <http://trac.webkit.org/changeset/191187>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.