Closed Bug 1353994 Opened 7 years ago Closed 7 years ago

stylo: Support new color format of hsla() and rgba() defined in CSS Color Module Level 4

Categories

(Core :: CSS Parsing and Computation, enhancement, P1)

Product:
Core
Component:
CSS Parsing and Computation
Platform:
x86_64
Linux
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: shinglyu, Assigned: kuoe0.tw)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Bug 1353994 - (Part 1) Update the version of cssparser to 0.12.2.
59 bytes, text/x-review-board-request
SimonSapin
: review+
heycam
: review+
Details
Bug 1353994 - (Part 2) Update test case result.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
Test cases like

   layout/reftests/w3c-css/submitted/color4/background-color-hsl-001.html

are failing because hsla() parsing only supports the following form:

   background-color: hsla(120, 75%, 50%, 0.4);

Messing around with the commas and change the 0.4 to 40% will break it.
Assignee: nobody → kuoe0
Looks like Tommy is actively working on this, marking P1.
Priority: -- → P1
Gecko support the new syntax of hsla defined in CSS Color Module Level 4[1]. But the cssparser[2] used in servo (stylo) doesn't support the new syntax and causes the test case failed. 

[1]: https://drafts.csswg.org/css-color-4/#the-hsl-notation
[2]: https://github.com/manuel-woelker/rust-cssparser
I found this pull request[1] fixed this bug, and already merged into cssparser few minutes ago. So, this bug will be fixed after servo updates cssparser module.

[1]: https://github.com/servo/rust-cssparser/issues/113
Great! Simon is publishing the new cssparser to crates.io now. Would you mind running |./mach vendor rust| and pushing the update?
Flags: needinfo?(kuoe0)
(You may need to cargo update -p cssparser as well, I'm not sure if mach vendor rust does an actual cargo update)
Thanks, Bobby!
Flags: needinfo?(kuoe0)
Attachment #8856871 - Flags: review?(simon.sapin)
Attachment #8856872 - Flags: review?(simon.sapin)
I tested locally and all test cases of color4 passed. But the try result is still running.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c36f78d3c5d6abae3490bcd9d1b4108df030737c
Comment on attachment 8856872 [details]
Bug 1353994 - (Part 2) Update test case result.

https://reviewboard.mozilla.org/r/128800/#review131310
Attachment #8856872 - Flags: review?(simon.sapin) → review+
Comment on attachment 8856871 [details]
Bug 1353994 - (Part 1) Update the version of cssparser to 0.12.2.

https://reviewboard.mozilla.org/r/128798/#review131312
Attachment #8856871 - Flags: review?(simon.sapin) → review+
For the record, this patch should be landed after this PR[1] mergerd.

[1]: https://github.com/servo/servo/pull/16343
Summary: stylo: hsla() color parsing not complete → stylo: Support new color format of hsla() and rgba() defined in CSS Color Module Level 4
This bug an that PR are completely independent. Either can merge without the other.
Attachment #8856872 - Attachment is obsolete: true
Remove the changes of reftest-stylo.list. The testcase still failed on try, and it is related to the rounding issue (See Bug 1340484).
Keywords: checkin-needed
Keywords: checkin-needed
Autoland can't push this until it has a review in MozReview from someone with Level 3 commit access.
Keywords: checkin-needed
Attachment #8856871 - Flags: review?(cam)
Attachment #8857285 - Flags: review?(cam)
Comment on attachment 8856871 [details]
Bug 1353994 - (Part 1) Update the version of cssparser to 0.12.2.

(This one doesn't need my review, since it'll land in the Servo PR.)
Attachment #8856871 - Flags: review?(cam)
Comment on attachment 8857285 [details]
Bug 1353994 - (Part 2) Update test case result.

https://reviewboard.mozilla.org/r/129232/#review131858
Attachment #8857285 - Flags: review?(cam) → review+
Comment on attachment 8856871 [details]
Bug 1353994 - (Part 1) Update the version of cssparser to 0.12.2.

https://reviewboard.mozilla.org/r/128798/#review131936
Attachment #8856871 - Flags: review+
Keywords: checkin-needed
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 b5e44d64fa59 -d 819f8b4c80b6: rebasing 389419:b5e44d64fa59 "Bug 1353994 - (Part 1) Update the version of cssparser to 0.12.2. r=heycam,SimonSapin"
merging toolkit/library/gtest/rust/Cargo.lock
merging toolkit/library/rust/Cargo.lock
rebasing 389420:d9137dd3e1a4 "Bug 1353994 - (Part 2) Update test case result. r=heycam" (tip)
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)
Keywords: checkin-needed
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 00e9c7d8ac5e -d f709675d6a79: rebasing 389737:00e9c7d8ac5e "Bug 1353994 - (Part 1) Update the version of cssparser to 0.12.2. r=heycam,SimonSapin"
rebasing 389738:f245281c0ac7 "Bug 1353994 - (Part 2) Update test case result. r=heycam" (tip)
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)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/eeebe28b1104
Part 1: Update the version of cssparser to 0.12.2. r=heycam, r=SimonSapin
https://hg.mozilla.org/integration/autoland/rev/82362b1c852d
Part 2: Update test case result. r=heycam
Keywords: checkin-needed
I took pity on your poor soul and did the rebase of Part 2 for you :P
https://hg.mozilla.org/mozilla-central/rev/eeebe28b1104
https://hg.mozilla.org/mozilla-central/rev/82362b1c852d
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.

Attachment

General

Creator:
Created:
Updated:
Size: