1457333 - Purge -moz- and -webkit-prefixed values from completion list in devtools

Status: NEW

(DevTools :: Inspector: Rules, enhancement, P3)

Description

[Xidorn Quan [:xidorn] UTC+11]

Currently we list all available values from InspectorUtils::GetCSSValuesForProperty, but we probably want to remove -moz- and -webkit-prefixed values from that since we shouldn't be encouraging people using them.

Comment 1

[Xidorn Quan [:xidorn] UTC+11]

Attachment: Bug 1457333 - Purge -moz- and -webkit-prefixed values from completion list in devtools.

Review commit: https://reviewboard.mozilla.org/r/251164/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/251164/

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8985553 [details]
Bug 1457333 - Purge -moz- and -webkit-prefixed values from completion list in devtools.

https://reviewboard.mozilla.org/r/251164/#review257414

::: devtools/shared/css/generated/properties-db.js
(Diff revision 1)
>      "supports": [],
>      "values": [
> -      "-moz-available",
> -      "-moz-fit-content",
> -      "-moz-max-content",
> -      "-moz-min-content",

I guess it's somewhat unfortunate to remove prefixed values with no unprefixed alternative, but I guess it's fine.

::: servo/components/style_derive/specified_value_info.rs:85
(Diff revision 1)
>      if let Some(other_values) = info_attrs.other_values {
>          for value in other_values.split(",") {
>              values.push(value.to_string());
>          }
>      }
> +    values.retain(|v| !v.starts_with("-moz-") && !v.starts_with("-webkit-"));

nit: Maybe add a comment?

Comment 3

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8985553 [details]
Bug 1457333 - Purge -moz- and -webkit-prefixed values from completion list in devtools.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/251164/diff/1-2/

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

Comment on attachment 8985553 [details]
Bug 1457333 - Purge -moz- and -webkit-prefixed values from completion list in devtools.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/251164/diff/2-3/

Comment 5

[Tom Tromey :tromey]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #2)

> > -      "-moz-available",
> > -      "-moz-fit-content",
> > -      "-moz-max-content",
> > -      "-moz-min-content",
> 
> I guess it's somewhat unfortunate to remove prefixed values with no
> unprefixed alternative, but I guess it's fine.

I think it would be good if either :gl or :pbro weighed in on this.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Fwiw, those in particular are going to be unprefixed soon (bug 1322780 came up on the list of stuff to do this quarter).

Comment 7

[Tom Tromey :tromey]

Comment on attachment 8985553 [details]
Bug 1457333 - Purge -moz- and -webkit-prefixed values from completion list in devtools.

https://reviewboard.mozilla.org/r/251164/#review257732

I have no issues with the patch per se, but as mentioned in the comments, I think it would be good to check in with gl or pbro before landing.  Perhaps it is still worth keeping these completions since some people may be using the prefixed forms by preference?  I am not really sure.

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

I think this is fine so long we are going to unprefix them soon. Perhaps hold off for 62 and land for 63. This might be a good time to bring up if we have any way of knowing the usage of the prefixed version of these property values in FF. So, we can make better decision of how and when to go about unprefixing in the future for the devtools autocomplete.

Comment 9

[Patrick Brosset <:pbro>]

Happy with whatever :gl is happy with.

Comment 10

[Xidorn Quan [:xidorn] UTC+11]

Having bug 1322780 block this given the concern in comment 2 about removing prefixed keywords without equivalents.

Comment 11

[ExE Boss]

Bug 1128200 suggests only hiding the prefixed values from the completion list if there’s no prefix‑less alternative.

Comment 13

[Tim Nguyen :ntim]

I personally think some prefixed values/properties are useful for Firefox Engineers, especially in the Browser Toolbox, but I agree hiding them for websites is a good idea.

Comment 14

[Sebastian Zartner [:sebo]]

(In reply to Tim Nguyen :ntim (please use needinfo?) from comment #13)
> I personally think some prefixed values/properties are useful for Firefox
> Engineers, especially in the Browser Toolbox, but I agree hiding them for
> websites is a good idea.

Those, which don't have a standardized counterpart implemented (yet) like -moz-appearance or -moz-outline-radius or the -moz-element value for background-image, should be kept in the completion lists, even for websites. But for all that do have one, I totally agree they should be hidden.

Sebastian

Comment 15

[Xidorn Quan [:xidorn] UTC+11]

(In reply to Sebastian Zartner [:sebo] from comment #14)
> (In reply to Tim Nguyen :ntim (please use needinfo?) from comment #13)
> > I personally think some prefixed values/properties are useful for Firefox
> > Engineers, especially in the Browser Toolbox, but I agree hiding them for
> > websites is a good idea.
> 
> Those, which don't have a standardized counterpart implemented (yet) like
> -moz-appearance or -moz-outline-radius or the -moz-element value for
> background-image, should be kept in the completion lists, even for websites.
> But for all that do have one, I totally agree they should be hidden.

I don't agree. We should not encourage people to use values which are not on the standard track, since they are not going to be webcompat.

In your examples, I think it has been clear that we want to remove -moz-outline-radius, and use border-radius for outline as well. And -moz-element is probably something we may have in some spec at some point, but it may not look this way. The situation of -moz-appearance is more complicated and I don't have suggestion without further investigation, but IIRC it is someting on the standard track and would be unprefixed or have standard webkit-prefix at some point.