The 'subgrid' feature is at-risk: https://drafts.csswg.org/css-grid/ Since we don't intend to support it in the initial release we should put the style system support we have behind a pref.

Comment on attachment 8686855 [details] [diff] [review] Patch 1: Pref out support for 'subgrid'. This looks great, thanks! A couple of nits: >layout/base/nsLayoutUtils.cpp >+nsLayoutUtils::IsGridTemplateSubgridValueEnabled() >+{ >+ static bool sGridTemplateSubgridValueEnabled; >+ static bool sTGridTemplateSubgridValueEnabledPrefCached = false; The first T in sTGrid... looks like a typo? Please remove that T so we follow the established naming scheme in surrounding code. >layout/style/nsCSSParser.cpp > if (ident->LowerCaseEqualsLiteral("subgrid")) { >+ if (!nsLayoutUtils::IsGridTemplateSubgridValueEnabled()) { >+ return false; >+ } I think it would be helpful to add an explicit error message in these places that explains to the author that we don't support 'subgrid' in this version. Something like: REPORT_UNEXPECTED(PESubgridNotSupported); with the associated message string: "The 'subgrid' keyword of CSS Grid isn't supported in this version." or something like that. >layout/style/test/property_database.js >+ if (IsCSSPropertyPrefEnabled("layout.css.grid-template-subgrid-value.enabled")) { >+ gCSSProperties["grid-template-columns"].other_values.push( > ... >+ gCSSProperties["grid-template-columns"].invalid_values.push( > ... Could you move the above two sections to just after the "gCSSProperties["grid-template-columns"] = ..." section? That way we will also include these into 'grid-template-rows' which follows and copies the values off -columns: http://mxr.mozilla.org/mozilla-central/source/layout/style/test/property_database.js#5864 >+ gCSSProperties["grid-templaye"].other_values.push( > ... Typo: s/templaye/template/ >+ gCSSProperties["grid-templaye"].invalid_values.push( Again. And move these two to just after the "gCSSProperties["grid-template"] =" section please.

I think it would be good idea to submit two separate Try jobs for testing: one with the patch as is, and one with the pref set to true. (in both cases, "-u mochitests,reftest" should be sufficient I think)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0aade5384ef with pref set to false (default) https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb4e6aae14b5 with pref set to true

Comment on attachment 8687392 [details] [diff] [review] Patch 1 (v2): Pref out support for 'subgrid'. Thanks, this look good to me. r=mats The Try jobs doesn't look good though. You we're probably just unlucky and used a bad revision as parent. Might be worth to rebase and resubmit?

Yeah, seams like the Try jobs look way better after a rebase: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e1bbc74a271 https://treeherder.mozilla.org/#/jobs?repo=try&revision=8358695f56f3