[css-grid] Should grid-template-areas accept empty strings?

CSS.supports("grid-template-areas", `""`)
CSS.supports("grid-template-areas", `"" ""`)

They are both true in Firefox and false in Chromium and old Edge. Looking at https://drafts.csswg.org/css-grid/#propdef-grid-template-areas,

There is no trash token.
All strings have the same number of columns (0).
There's in no non-rectangular named grid area (since there's no named grid area).

So it seems valid per the spec. But also pretty pointless, so possibly it was not deliberate. Given that neither Chromium nor old Edge support that, should we really allow it?

I guess the utility of this is something like

#grid {
  display: inline-grid;
  grid-auto-columns: 50px 100px;
  grid-auto-rows: 50px 100px;
  grid-template-areas: "";
  border: solid;
}

and if the grid is empty, you get a 50px tall and 0px wide grid. If you used grid-template-areas: "." you would get a 50x50 square. If you used grid-template-rows: 50px then you would have to reorder grid-auto-rows: 100px 50px in case there are two items, but this would also affect leading implicit tracks. Maybe grid-template-rows: 50px repeat(auto-fit, 100px) could be an alternative?

Anyways I don't know why somebody would want to do this, and allowing it for rows but not for columns is not much consistent either.

@rachelandrew

CC @rachelandrew and @jensimmons for an opinion...

@fantasai

@fantasai I am struggling to think of a use case, so don't have a strong feeling either way, other than we should probably clarify it and raise the issue with the relevant browser.

It definitely wasn't deliberate, but there's also no use-case for it, so whatever's easiest to spec/implement seems reasonable. I have no preference one way or the other.

Supporting this in Chromium would be simple (just removing some checks).

diff --git a/third_party/blink/renderer/core/css/css_grid_template_areas_value.cc b/third_party/blink/renderer/core/css/css_grid_template_areas_value.cc
index 57a7d1e672d..a9b7b2f7393 100644
--- a/third_party/blink/renderer/core/css/css_grid_template_areas_value.cc
+++ b/third_party/blink/renderer/core/css/css_grid_template_areas_value.cc
@@ -44,7 +44,6 @@ CSSGridTemplateAreasValue::CSSGridTemplateAreasValue(
       row_count_(row_count),
       column_count_(column_count) {
   DCHECK(row_count_);
-  DCHECK(column_count_);
 }
 
 static String StringForPosition(const NamedGridAreaMap& grid_area_map,
diff --git a/third_party/blink/renderer/core/css/properties/css_parsing_utils.cc b/third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
index 7e66f00f7f8..501e222ae75 100644
--- a/third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
+++ b/third_party/blink/renderer/core/css/properties/css_parsing_utils.cc
@@ -1638,7 +1638,6 @@ CSSIdentifierValue* ConsumeFontVariantCSS21(CSSParserTokenRange& range) {
 }
 
 Vector<String> ParseGridTemplateAreasColumnNames(const String& grid_row_names) {
-  DCHECK(!grid_row_names.IsEmpty());
   Vector<String> column_names;
   // Using StringImpl to avoid checks and indirection in every call to
   // String::operator[].
@@ -2053,15 +2052,10 @@ bool ParseGridTemplateAreasRow(const String& grid_row_names,
                                NamedGridAreaMap& grid_area_map,
                                const size_t row_count,
                                size_t& column_count) {
-  if (grid_row_names.ContainsOnlyWhitespaceOrEmpty())
-    return false;
-
   Vector<String> column_names =
       ParseGridTemplateAreasColumnNames(grid_row_names);
   if (row_count == 0) {
     column_count = column_names.size();
-    if (column_count == 0)
-      return false;
   } else if (column_count != column_names.size()) {
     // The declaration is invalid if all the rows don't have the number of
     // columns.
diff --git a/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc b/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
index 39164982348..80a5a649fca 100644
--- a/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
+++ b/third_party/blink/renderer/core/css/properties/longhands/longhands_custom.cc
@@ -3082,7 +3082,6 @@ const CSSValue* GridTemplateAreas::ParseSingleValue(
 
   if (row_count == 0)
     return nullptr;
-  DCHECK(column_count);
   return MakeGarbageCollected<cssvalue::CSSGridTemplateAreasValue>(
       grid_area_map, row_count, column_count);
 }

I'd be fine with that, though not a big fan of its asymmetry.

if the grid is empty, you get a 50px tall and 0px wide grid. If you used grid-template-areas: "." you would get a 50x50 square.

I'm a bit lost about why you think that should work like that. In your example if you have no items the grid container is 0x0 in Chromium/WebKit and 0x50 in Firefox (independently if you set grid-template-areas to "" or to "."). So it seems what you describe there is not happening in any browser right now.

@mrego

@mrego See https://software.hixie.ch/utilities/js/live-dom-viewer/saved/8150
grid-template-areas: "" is 0x50 in Firefox and 0x0 in Chromium.
grid-template-areas: "." is 50x50 everywhere, since it forces the explicit grid to have at least 1 column and 1 row, even if there is no item.

@emilio

Since nobody has a strong opinion either way, and nobody has use-cases for an explicitly empty g-t-a, I'm inclined to go with the majority implementation here, and rule that it's disallowed, so g-t-a must define at least one cell to be valid.

@emilio @MatsPalmgren any objection to this?

The CSS Working Group just discussed [css-grid] Should grid-template-areas accept empty strings?, and agreed to the following:

RESOLVED: grid-template-areas should not accept empty strings

The full IRC log of that discussion

<dael> Topic: [css-grid] Should grid-template-areas accept empty strings?
<dael> github: https://github.com//issues/5110
<dael> TabAtkins: Oriol raised issue that tech spec does not disallow empty strings in grid-template-areas
<dael> TabAtkins: No reason to do it either way but no one wants to put empty string in. Both Chrome and Edge consider it to be invalid
<dael> TabAtkins: As long as no objections we're going to say we require one cell to be expressed or it's invalid
<dael> Rossen_: Other opinions?
<dael> plinss: Req at least one cell with a name?
<dael> TabAtkins: A cell. It can be empty.
<dael> plinss: If you define an area with empty string for name what does it mean?
<dael> TabAtkins: It's got a track but no alignment
<dael> plinss: Same as dot?
<dael> florian: [missed] parser
<dael> TabAtkins: We don't give strings per cell name. String is name of all cells in row
<dael> plinss: Right
<florian> s/[missed] parser/no, empty string would be parse error/
<dael> oriol: Difference between empty stirng and string with dot is it forces it to have spec number of column and rows. If allow empty strings grid would force 5 rows and no column
<dael> oriol: Strings with dots you could have 1 column. That's difference
<dael> TabAtkins: Given no use case for rows with 0 columns and it doesn't go the other way around I'm inclined to call invalid matching Chrome and Edge
<dael> florian: Doesn't seem to be a use case to allow. It's a weird error and we can align on the market share of browsers
<dael> Rossen_: I can see it spit out by preprocessors or tools. Having spec is good
<dael> Rossen_: I don't hear obj or pushback.
<dael> Rossen_: Other questions or reasons why we shouldn't?
<florian> s/on the market share of browsers/, the market share of browsers who do this already prove it's possible/
<dael> dholbert: Sounds fine to me/Mozilla
<dael> RESOLVED: grid-template-areas should not accept empty strings

I don't feel strongly either way, so this seems fine to me. I'm assuming the resolution is not just about empty strings, but also that strings containing only whitespace are also invalid. IOW, a string that doesn't produce any null or named cell tokens is invalid.

Yes, it's actually a requirement that g-t-a must define at least one cell. This'll be clear when I make the edits.

@tabatkins

@tabatkins : just to be sure we're understanding...

Can you clarify which of these values should be considered invalid:

grid-template-areas: "";
grid-template-areas: "" "";
grid-template-areas: "foo" "";

Are all three invalid? (I think that would make sense, and it's how Mats has implemented the change in a pending patch, but I'm not sure it agrees with your "must define at least one cell" phrasing above... It sounds like your phrasing might allow the third example here.)

@dholbert

@dholbert The 3rd example must be invalid per

All strings must have the same number of columns, or else the declaration is invalid.

I like that, but I'm not sure it jives with Tab's "must define at least one cell" phrasing of the requirement.

Ah, I see -- I suppose that third example does define one cell, but then fails a separate requirement about number of columns having to match between each string, so it still fails. Sounds good to me!

Correct.

[css-grid-1] Require grid-template-areas to define at least one grid …

Tested by web-platform-tests/wpt@31bf745

Navigation Menu

Repository navigation

[css-grid] Should grid-template-areas accept empty strings? #5110

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[css-grid] Should grid-template-areas accept empty strings? #5110

Description

Activity

Loirooriol commented on May 25, 2020

fantasai commented on May 25, 2020

rachelandrew commented on May 27, 2020

tabatkins commented on May 28, 2020

Loirooriol commented on May 28, 2020

mrego commented on May 29, 2020

Loirooriol commented on May 29, 2020

tabatkins commented on Jul 7, 2020

css-meeting-bot commented on Jul 15, 2020

MatsPalmgren commented on Jul 16, 2020

tabatkins commented on Jul 17, 2020

dholbert commented on Jul 21, 2020

Loirooriol commented on Jul 21, 2020

dholbert commented on Jul 21, 2020

dholbert commented on Jul 21, 2020

tabatkins commented on Jul 22, 2020

tabatkins commented on Jul 23, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions