[css-grid] Update <fixed-size> parsing to the latest spec

RESOLVED FIXED in Firefox 47

Status

()

Core
CSS Parsing and Computation
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

unspecified
mozilla47
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
The syntax for <fixed-size> recently changed to:
<fixed-size> = 
  <fixed-breadth> | 
  minmax( <fixed-breadth> , <track-breadth> ) |
  minmax( <track-breadth> , <fixed-breadth> )
https://drafts.csswg.org/css-grid/#typedef-fixed-size

I think our layout code already handles this correctly though - we use
the max-sizing function if it's definite, otherwise the min-sizing.

I should update the code comment here though since the spec text changed:
http://hg.mozilla.org/mozilla-central/annotate/5e0140b6d118/layout/generic/nsGridContainerFrame.cpp#l428
(Assignee)

Updated

2 years ago
Priority: -- → P3
(Assignee)

Comment 1

2 years ago
Created attachment 8724826 [details] [diff] [review]
fix
Attachment #8724826 - Flags: review?(dholbert)
Keywords: dev-doc-needed
Comment on attachment 8724826 [details] [diff] [review]
fix

Review of attachment 8724826 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for being on top of this spec-change!

r=me, just one nit:

::: layout/style/nsCSSParser.cpp
@@ +8603,5 @@
>  CSSParseResult
>  CSSParserImpl::ParseGridTrackSize(nsCSSValue& aValue,
>                                    GridTrackSizeFlags aFlags)
>  {
> +  const bool requireFixedSize = !!(aFlags & GridTrackSizeFlags::eFixedTrackSize);

This line is a little too long -- either wrap after the "=", or do something like s/require/need/ in the variable-name to shorten.
Attachment #8724826 - Flags: review?(dholbert) → review+
I skimmed the tests, too, & they seem sane - feel free to mark those as rs=dholbert if you like

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bd98615e4b4b
https://hg.mozilla.org/mozilla-central/rev/9060d17ed510
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.