Closed Bug 1005567 Opened 7 years ago Closed 7 years ago

[css-grid] Remove grid-auto-position

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: mats, Assigned: mats)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attachment #8416973 - Flags: review?(simon.sapin)
Comment on attachment 8416973 [details] [diff] [review]
fix

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

Looks good, except:

::: layout/style/test/property_database.js
@@ +5176,5 @@
>  			"grid-row-end"
>  		],
>  		initial_values: [ "auto", "auto / auto" ],
> +		other_values: gridLineOtherValues,
> +		invalid_values: gridLineInvalidValues

This (and above for grid-column) is removing tests that should not be removed (i.e. values with two explicit <grid-line>s.) Please keep gridColumnRowOtherValues and gridColumnRowInvalidValues with the same content as before, maybe pushing to them directly without the (now unused) gridAutoPosition* intermediate variables.
Attachment #8416973 - Flags: review?(simon.sapin) → review-
Attached patch fixSplinter Review
Good catch, thanks.  Is this what you suggested?  I dropped the "foo"
in the array - I suppose that's what the splice removed?

Looks green (the orange is unrelated):
https://tbpl.mozilla.org/?tree=Try&rev=ee4f50ddd48f
Attachment #8417848 - Flags: review?(simon.sapin)
Attachment #8416973 - Attachment is obsolete: true
Comment on attachment 8417848 [details] [diff] [review]
fix

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

Looks good, thanks!
Attachment #8417848 - Flags: review?(simon.sapin) → review+
https://hg.mozilla.org/mozilla-central/rev/7f715a1d5b28
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
This is still disabled by default, won't affect site compat.
You need to log in before you can comment on or make changes to this bug.