Closed
Bug 1355402
Opened 7 years ago
Closed 7 years ago
stylo: Support prefixed intrinsic size value for {min-,max-,}{width,height,{inline,block}-size}
Categories
(Core :: CSS Parsing and Computation, enhancement, P1)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: xidorn, Assigned: hiro)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 5 obsolete files)
Currently, Stylo supports unprefixed intrinsic size keyword values for {min,max}-*, while Gecko supports (and only supports) -moz-prefixed keyword values. Also Stylo doesn't support that for width/height/{inline,block}-size.
Updated•7 years ago
|
Priority: -- → P2
Updated•7 years ago
|
Assignee: nobody → slyu
Updated•7 years ago
|
Priority: P2 → P1
Comment 1•7 years ago
|
||
Xidorn, does this include support for -moz-available, or should that be a separate bug?
Flags: needinfo?(xidorn+moz)
Updated•7 years ago
|
Blocks: stylo-reftest
Comment 2•7 years ago
|
||
Thanks Shing! bz says this is high priority, so let me know if you get stuck or don't have time to work on it.
Reporter | ||
Comment 3•7 years ago
|
||
I think this bug should probably include all the prefixed keyword values width-ish properties support, including -moz-{{min,max,fit}-content,available}. Is -moz-available somehow particularly interesting and should be done in a separate bug?
Flags: needinfo?(xidorn+moz)
Comment 4•7 years ago
|
||
No, it was just me being confused by the summary of this bug, which refers to property _names_, not values.
Assignee | ||
Comment 6•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=420e03526bf15ed676a0b2ee466c2a1dff216567
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8868280 [details] Bug 1355402 - Combine LengthOrPercentage and Auto into LengthOrPercentageOrAuto for {Min,Max}Length. https://reviewboard.mozilla.org/r/139868/#review143222
Attachment #8868280 -
Flags: review?(manishearth) → review+
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8868281 [details] Bug 1355402 - Update test expectations for -moz-{min,max,fit}-content,available}. https://reviewboard.mozilla.org/r/139870/#review143224
Attachment #8868281 -
Flags: review?(manishearth) → review+
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8868282 [details] Bug 1355402 - Support prefixed intrinsic size value for flex-basis. https://reviewboard.mozilla.org/r/139872/#review143226
Attachment #8868282 -
Flags: review?(manishearth) → review+
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8868283 [details] Bug 1355402 - Update mochitest expectations for prefixed intrinsic size value of flex-basis. https://reviewboard.mozilla.org/r/139874/#review143232
Attachment #8868283 -
Flags: review?(manishearth) → review+
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8868280 [details] Bug 1355402 - Combine LengthOrPercentage and Auto into LengthOrPercentageOrAuto for {Min,Max}Length. https://reviewboard.mozilla.org/r/139868/#review143230 ::: servo/components/style/properties/longhand/position.mako.rs:157 (Diff revision 1) > if logical: > spec = "https://drafts.csswg.org/css-logical-props/#propdef-%s" > %> > // width, height, block-size, inline-size > + % if product == "gecko": > + ${helpers.predefined_type("%s" % size, We can't use a predefined type here. The ToComputedValue impl, like that of Min/MaxSize, needs to filter out kw values in the block direction. We should specify newtypes for each one (like done for Min/MaxSize with `struct SpecifiedValue(MaxLength)`), with custom parse and ToComputedValue impls. They should share the same computed value type (`computed::MozLength`).
Attachment #8868280 -
Flags: review+ → review-
Assignee | ||
Comment 16•7 years ago
|
||
(In reply to Manish Goregaokar [:manishearth] from comment #15) > Comment on attachment 8868280 [details] > Bug 1355402 - Support prefixed intrinsic size value for > {min-,max-,}{width,height,{inline,block}-size}}. > > https://reviewboard.mozilla.org/r/139868/#review143230 > > ::: servo/components/style/properties/longhand/position.mako.rs:157 > (Diff revision 1) > > if logical: > > spec = "https://drafts.csswg.org/css-logical-props/#propdef-%s" > > %> > > // width, height, block-size, inline-size > > + % if product == "gecko": > > + ${helpers.predefined_type("%s" % size, > > We can't use a predefined type here. The ToComputedValue impl, like that of > Min/MaxSize, needs to filter out kw values in the block direction. > > We should specify newtypes for each one (like done for Min/MaxSize with > `struct SpecifiedValue(MaxLength)`), with custom parse and ToComputedValue > impls. They should share the same computed value type > (`computed::MozLength`). Oh, thanks. I did not notice it at all. I will check Min/MaxSize. Thanks for the pointer!
Assignee | ||
Comment 17•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=532d230e4bc694f03d2bc396d083e6bdd03a0cdd
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 25•7 years ago
|
||
Comment on attachment 8868282 [details] Bug 1355402 - Support prefixed intrinsic size value for flex-basis. This one needs to be re-reviewed.
Attachment #8868282 -
Flags: review+ → review?(manishearth)
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8868280 [details] Bug 1355402 - Combine LengthOrPercentage and Auto into LengthOrPercentageOrAuto for {Min,Max}Length. https://reviewboard.mozilla.org/r/139868/#review144792
Attachment #8868280 -
Flags: review?(manishearth) → review+
Comment 27•7 years ago
|
||
mozreview-review |
Comment on attachment 8868381 [details] Bug 1355402 - Rename MinLength to MozLength. https://reviewboard.mozilla.org/r/139960/#review144796 ::: servo/components/style/values/computed/length.rs:564 (Diff revision 1) > pub type LengthOrNumber = Either<Length, Number>; > > /// Either a computed `<length>` or the `normal` keyword. > pub type LengthOrNormal = Either<Length, Normal>; > > /// A value suitable for a `min-width` or `min-height` property. also width and height
Attachment #8868381 -
Flags: review?(manishearth) → review+
Comment 28•7 years ago
|
||
mozreview-review |
Comment on attachment 8868382 [details] Bug 1355402 - Factor out implemantations for {min,max} size properties as a macro. https://reviewboard.mozilla.org/r/139962/#review144798 ::: servo/components/style/properties/helpers.mako.rs:1038 (Diff revision 1) > } > </%def> > + > +// Define property that supports prefixed intrinsic size keyword values for gecko. > +// E.g. -moz-max-content, -moz-min-content, etc. > +<%def name="gecko_length_type(name, length_type, initial_value, logical, **kwargs)"> I would call this gecko_size_type
Attachment #8868382 -
Flags: review?(manishearth) → review+
Comment 29•7 years ago
|
||
mozreview-review |
Comment on attachment 8868383 [details] Bug 1355402 - Support prefixed intrinsic size value for {width,height,{inline,block}-size}}. https://reviewboard.mozilla.org/r/139964/#review144802
Attachment #8868383 -
Flags: review?(manishearth) → review+
Comment 30•7 years ago
|
||
mozreview-review |
Comment on attachment 8868282 [details] Bug 1355402 - Support prefixed intrinsic size value for flex-basis. https://reviewboard.mozilla.org/r/139872/#review144804
Attachment #8868282 -
Flags: review?(manishearth) → review+
Assignee | ||
Comment 31•7 years ago
|
||
Thanks for the review! https://github.com/servo/servo/pull/16962
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8868280 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8868381 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8868382 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8868383 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8868282 -
Attachment is obsolete: true
Comment 34•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 074a5a7d97d8 -d 8a0798c5e404: rebasing 397530:074a5a7d97d8 "Bug 1355402 - Update test expectations for -moz-{min,max,fit}-content,available}. r=manishearth" merging layout/generic/crashtests/crashtests.list merging layout/reftests/bugs/reftest.list merging layout/style/test/stylo-failures.md warning: conflicts while merging layout/style/test/stylo-failures.md! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 37•7 years ago
|
||
Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ab4e65106302 Update test expectations for -moz-{min,max,fit}-content,available}. r=manishearth https://hg.mozilla.org/integration/autoland/rev/ceaa661a7fdb Update mochitest expectations for prefixed intrinsic size value of flex-basis. r=manishearth
Comment 38•7 years ago
|
||
Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ef82929381b2 Update reftest expectations. r=me
Comment 39•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ab4e65106302 https://hg.mozilla.org/mozilla-central/rev/ceaa661a7fdb https://hg.mozilla.org/mozilla-central/rev/ef82929381b2
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•