There are several -moz-prefixed XUL-specific display values exposed to web content, which are not speced properly, and we do not have enough test coverage on them with other features either. Exposing them could cause compatibility issues e.g. bug 1288472. We should exclude them from display ktable when parsing CSS for web content.

I guess we should make `display` use parsing function, rather than keeping it in the normal keyword path and hacking in the normal path like what we are currently doing.

Thanks for filing this. (Along with display:-moz-box, I expect XUL's "display:-moz-grid" to inadvertantly cause compat problems similar to bug 1288472, as modern CSS grid gets enabled & it gains mindshare, and as authors ask for it in the wrong way, & with declarations in the wrong order)

I wonder how much we might break, for old sites using display: -moz-box, but without a display: flex. But maybe our webkit flexy display aliases let us get away with it?

Is -moz-box ever expected to work in web content? I don't believe so... I think they are used just because people see there is something with that name, and they think it is the Mozilla version of -webkit-box...

> Is -moz-box ever expected to work in web content? I don't believe so... I think they are used just because people see there is something with that name, and they think it is the Mozilla version of -webkit-box... Yeah, I agree that's probably the reasoning so many people included it. But I suspect there's enough content like the following (crappy first example I found on GitHub search): https://github.com/obashtovoj/5course/blob/a958f8d34e1dd9b2e514ca142722418f77dd8a59/Web-programming/HTML5/HTML5/PropertiesOfFlexibleBoxModel/2_46.css Now, if we removed -moz-box support, the -webkit-box alias stuff would kick in and nothing would break. But I wouldn't be surprised by sites doing UA sniffing to only serve old -moz-box. (Of course, this is just paranoid speculation. We could implement it behind a pref and surf the web and see what happens.)

As I said in bug 914360, I think we can unexpose the following values: -moz-grid -moz-inline-grid -moz-grid-group -moz-grid-line -moz-stack -moz-inline-stack -moz-deck -moz-popup -moz-groupbox I'm less sure about -moz-box / -moz-inline-box though. I suggest we wait with those and gather some telemetry about their use.

Comment on attachment 8982840 [details] Bug 1288572: Don't hide -moz-box / -moz-inline-box yet. https://reviewboard.mozilla.org/r/248702/#review254898 ::: commit-message-440c3:3 (Diff revision 1) > +Bug 1288572: Don't hide -moz-box / -moz-inline-box yet. r?mats > + > +I'd really prefer to not land this patch, but... Well, I guess it's the right call, and reduces the risk of unshipping the other values. Looks fine to do this for now :)

Comment on attachment 8982840 [details] Bug 1288572: Don't hide -moz-box / -moz-inline-box yet. https://reviewboard.mozilla.org/r/248702/#review254900

Comment on attachment 8982833 [details] Bug 1288572: Introduce css(parse_condition). https://reviewboard.mozilla.org/r/248698/#review254938 ::: servo/components/style_derive/to_css.rs:238 (Diff revision 1) > pub function: Option<Override<String>>, > pub comma: bool, > pub dimension: bool, > pub keyword: Option<String>, > pub aliases: Option<String>, > + pub parse_condition: Option<Path>, Maybe we should add a struct for `Parse` deriving. `parse_condition` doesn't seem to be related to `ToCss` at all. Also, we should probably have comment on `parser::Parse` stating that the trait can be derived for some types, and it uses css attributes and it has extra attribute to control the condition.

Comment on attachment 8982834 [details] Bug 1288572: Hide -moz- display values from content behind a pref. https://reviewboard.mozilla.org/r/248700/#review254940 ::: servo/components/style/values/specified/box.rs:22 (Diff revision 2) > use values::generics::box_::VerticalAlign as GenericVerticalAlign; > use values::specified::{AllowQuirks, Number}; > use values::specified::length::{LengthOrPercentage, NonNegativeLength}; > > +#[cfg(feature = "gecko")] > +fn moz_display_values_enabled_on_content(context: &ParserContext) -> bool { `moz_display_values_enabled` should be enough... Having `on_content` makes it more confusing to me... ::: servo/components/style/values/specified/box.rs:26 (Diff revision 2) > + context.chrome_rules_enabled() || > + unsafe { > + structs::StaticPrefs_sVarCache_layout_css_xul_display_values_content_enabled > + } Indentations? I bet rustfmt doesn't like this.

Comment on attachment 8982867 [details] Bug 1288572: Update test expectations. https://reviewboard.mozilla.org/r/248722/#review254942 ::: layout/base/crashtests/crashtests.list:362 (Diff revision 2) > load 560441-1.xhtml > load 560447-1.html > load 564063-1.html > load 567292-1.xhtml > load 569018-1.html > -load 570038-1.html > +asserts(4) load 570038-1.html Maybe just pref on the preference for this test as well... When we eventually remove that pref and unship the values completely, this test can probably be removed as well.

Comment on attachment 8982833 [details] Bug 1288572: Introduce css(parse_condition). https://reviewboard.mozilla.org/r/248698/#review254938 > Maybe we should add a struct for `Parse` deriving. `parse_condition` doesn't seem to be related to `ToCss` at all. > > Also, we should probably have comment on `parser::Parse` stating that the trait can be derived for some types, and it uses css attributes and it has extra attribute to control the condition. Per that reasoning, we should probably also move `aliases` to this new attribute. If you agree on moving both, mind if I do it as a followup bug?

Comment on attachment 8982833 [details] Bug 1288572: Introduce css(parse_condition). (See comment 20)

Comment on attachment 8982833 [details] Bug 1288572: Introduce css(parse_condition). https://reviewboard.mozilla.org/r/248698/#review254938 > Per that reasoning, we should probably also move `aliases` to this new attribute. If you agree on moving both, mind if I do it as a followup bug? Okay.

Comment on attachment 8982833 [details] Bug 1288572: Introduce css(parse_condition). https://reviewboard.mozilla.org/r/248698/#review254990 OK, as far as you're splitting the attributes in a followup bug, and adding the document for this, r=me.

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/dba6641c8d1a Introduce css(parse_condition). r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/cf368e565ab8 Hide -moz- display values from content behind a pref. r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/ab37c2da6d45 Don't hide -moz-box / -moz-inline-box yet. r=mats https://hg.mozilla.org/integration/mozilla-inbound/rev/dd2ab05ec584 Update test expectations. r=xidorn

Posted the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/most-of-non-standard-css-display-values-have-been-dropped/

https://hg.mozilla.org/mozilla-central/rev/dba6641c8d1a https://hg.mozilla.org/mozilla-central/rev/cf368e565ab8 https://hg.mozilla.org/mozilla-central/rev/ab37c2da6d45 https://hg.mozilla.org/mozilla-central/rev/dd2ab05ec584

Browser compat data is updated for this: https://developer.mozilla.org/en-US/docs/Web/CSS/::selection#Browser_compatibility and I think that's all the doc update needed here, but please let me know if we need anything else.

Sorry, wrong bug :/. Anyway, ExE-Boss updated the docs for this: https://developer.mozilla.org/en-US/docs/Web/CSS/display#XUL_values so I think this one is also dev-doc-complete.

The Browser Compatibility table fix is however still pending in https://github.com/mdn/browser-compat-data/pull/1955.