Bug 127616 - Web Inspector: Add "unset" to CSS value autocompletion
Summary: Web Inspector: Add "unset" to CSS value autocompletion
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Minor
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on: 148614
Blocks:
  Show dependency treegraph
 
Reported: 2014-01-25 03:23 PST by Diego Pino
Modified: 2015-10-16 11:21 PDT (History)
9 users (show)

See Also:


Attachments
Patch (2.51 KB, patch)
2014-01-25 03:27 PST, Diego Pino
joepeck: review-
Details | Formatted Diff | Diff
Test of initial and inherit values on non-inherited property (background-image) (1.88 KB, text/html)
2014-01-25 17:09 PST, Diego Pino
no flags Details
[PATCH] Proposed Fix (5.17 KB, patch)
2015-10-15 21:38 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Pino 2014-01-25 03:23:19 PST
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
Comment 1 Radar WebKit Bug Importer 2014-01-25 03:23:28 PST
<rdar://problem/15908963>
Comment 2 Diego Pino 2014-01-25 03:27:06 PST
Created attachment 222204 [details]
Patch
Comment 3 Timothy Hatcher 2014-01-25 08:42:37 PST
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.
Comment 4 Joseph Pecoraro 2014-01-25 10:08:55 PST
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 =).
Comment 5 Joseph Pecoraro 2014-01-25 10:11:26 PST
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.
Comment 6 Diego Pino 2014-01-25 17:09:06 PST
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.
Comment 7 Joseph Pecoraro 2014-01-25 19:25:13 PST
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.
Comment 8 Joseph Pecoraro 2015-10-15 20:36:44 PDT
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.
Comment 9 Joseph Pecoraro 2015-10-15 21:10:08 PDT
Seems easy enough!
Comment 10 Joseph Pecoraro 2015-10-15 21:38:39 PDT
Created attachment 263245 [details]
[PATCH] Proposed Fix
Comment 11 Blaze Burg 2015-10-16 10:47:11 PDT
Comment on attachment 263245 [details]
[PATCH] Proposed Fix

r=me
Comment 12 WebKit Commit Bot 2015-10-16 11:21:16 PDT
Comment on attachment 263245 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 263245

Committed r191187: <http://trac.webkit.org/changeset/191187>
Comment 13 WebKit Commit Bot 2015-10-16 11:21:21 PDT
All reviewed patches have been landed.  Closing bug.