1274526 - Support 'q' absolute length units from CSS3 values and units

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Comment 1

[Edgar Chen [:edgar] (PTO until 02/09)]

https://www.w3.org/TR/css-values/#absolute-lengths

unit  | name                  | equivalence 
------+-----------------------+--------------------
q     | quarter-millimeters   | 1q = 1/40th of 1cm

Comment 2

[Edgar Chen [:edgar] (PTO until 02/09)]

I have a WIP patch, will attach it soon.

Comment 3

[Edgar Chen [:edgar] (PTO until 02/09)]

Attachment: WIP Patch, v1

Comment 4

[Edgar Chen [:edgar] (PTO until 02/09)]

Test case: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=4221
Expect result: all div block should have the exact same width and height.

Comment 5

[Edgar Chen [:edgar] (PTO until 02/09)]

Attachment: Part 1: Implement support for 'q' absolute length units in CSS, v2

Comment 6

[Edgar Chen [:edgar] (PTO until 02/09)]

Attachment: Part 2: Update web-platform-test expectation for parse-a-sizes-attribute.html, v1

Comment 7

[Edgar Chen [:edgar] (PTO until 02/09)]

Comment on attachment 8755254 [details] [diff] [review]
Part 1: Implement support for 'q' absolute length units in CSS, v2

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

Add support for 'q' absolute length unit, see spec https://www.w3.org/TR/css-values/#absolute-lengths.
And also add test case for 'q' unit in test_pixel_lengths.html.

Hi Cameron, may I have your review? Thank you.

Comment 8

[Edgar Chen [:edgar] (PTO until 02/09)]

Comment on attachment 8755255 [details] [diff] [review]
Part 2: Update web-platform-test expectation for parse-a-sizes-attribute.html, v1

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

Patch part1 adds the support for 'q' length unit, so we can pass some tests in parse-a-sizes-attribute.html.
Hi Josh, may I have your review? Thank you.

Comment 9

[Cameron McCormack (:heycam)]

Comment on attachment 8755254 [details] [diff] [review]
Part 1: Implement support for 'q' absolute length units in CSS, v2

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

This looks fine, although I wonder if it's important to support the "q" unit.  It is in the CSS Values & Units CR, but as far as I know no other browser implements it.  I see it's the last length unit from css-values-3 that we don't support, though, and it's easy, so I guess there's little downside.

::: layout/style/nsCSSValue.h
@@ +364,5 @@
>    eCSSUnit_Millimeter   = 902,    // (float) 96/25.4 CSS pixels
>    eCSSUnit_Centimeter   = 903,    // (float) 96/2.54 CSS pixels
>    eCSSUnit_Pica         = 904,    // (float) 12 points == 16 CSS pixls
>    eCSSUnit_Pixel        = 905,    // (float) CSS pixel unit
> +  eCSSUnit_Quarter      = 906,    // (float) 96/101.6 CSS pixels

In a way I think it's nice to have eCSSUnit_Pixel be the last one in this group of absolute lengths.  Can you make eCSSUnit_Quarter = 905 and eCSSUnit_Pixel = 906?

Comment 10

[Edgar Chen [:edgar] (PTO until 02/09)]

(In reply to Cameron McCormack (:heycam) from comment #9)
> Comment on attachment 8755254 [details] [diff] [review]
> Part 1: Implement support for 'q' absolute length units in CSS, v2
> 
> Review of attachment 8755254 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks fine, although I wonder if it's important to support the "q"
> unit.  It is in the CSS Values & Units CR, but as far as I know no other
> browser implements it.  I see it's the last length unit from css-values-3
> that we don't support, though, and it's easy, so I guess there's little
> downside.
> 
> ::: layout/style/nsCSSValue.h
> @@ +364,5 @@
> >    eCSSUnit_Millimeter   = 902,    // (float) 96/25.4 CSS pixels
> >    eCSSUnit_Centimeter   = 903,    // (float) 96/2.54 CSS pixels
> >    eCSSUnit_Pica         = 904,    // (float) 12 points == 16 CSS pixls
> >    eCSSUnit_Pixel        = 905,    // (float) CSS pixel unit
> > +  eCSSUnit_Quarter      = 906,    // (float) 96/101.6 CSS pixels
> 
> In a way I think it's nice to have eCSSUnit_Pixel be the last one in this
> group of absolute lengths.  Can you make eCSSUnit_Quarter = 905 and
> eCSSUnit_Pixel = 906?

Will do. Thank you.

Comment 11

[Edgar Chen [:edgar] (PTO until 02/09)]

Attachment: Part 1: Implement support for 'q' absolute length units in CSS, v3, r=heycam

Address review comment #9.

Comment 12

[Edgar Chen [:edgar] (PTO until 02/09)]

Attachment: Part 2: Update web-platform-test expectation for parse-a-sizes-attribute.html, v2, r=heycam

Just update reviewer field to 'r=heycam' in commit message.

Comment 13

[Edgar Chen [:edgar] (PTO until 02/09)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3d20da3b1ab02117f754401cf292fb3dd6d986f&group_state=expanded

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/929092ee1097
https://hg.mozilla.org/integration/mozilla-inbound/rev/bfa847fd5f53

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/929092ee1097
https://hg.mozilla.org/mozilla-central/rev/bfa847fd5f53

Comment 16

[Sebastian Zartner [:sebo]]

Updated documentation here:
https://developer.mozilla.org/en-US/docs/Web/CSS/length

And added a developer release info here:
https://developer.mozilla.org/en-US/Firefox/Releases/49#CSS

Sebastian