Details

Reviewers

Group Reviewers

Restricted Project

Commits

rFIREFOXAUTOLANDbf673891ceb2: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio
rFIREFOXAUTOLANDf360400c2978: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio

Bugzilla Bug ID

1965029

Summary

The alpha and colorspace part of the https://html.spec.whatwg.org/#serialize-a-color-well-control-color is largely skipped in this patch, as we don't support them yet in <input type=color>, and thus the value is always in the "HTML compatible" form.

Changing the existing WPT color.tentative.html to color.html to specifically cover implementations without alpha and colorspace support, because otherwise reading the test result of color.window.js is overly confusing. (Ideally the two tests should share the same test data, but for now I'm skipping that as I'm not confident yet to fill the display-p3 fields. That can be done when we actually implement colorspace support.)

Introducing Servo_ComputeColorWellControlColor so that the colorspace conversion can happen before losing number precision, following the spec.

Diff Detail

Repository

rFIREFOXAUTOLAND firefox-autoland

Build Status

Buildable 829372
Build 929941: arc lint + arc unit

Event Timeline

saschanaz planned changes to this revision.Jul 25 2025, 1:57 AM

saschanaz created this revision.

Herald added a project: secure-revision. · View Herald TranscriptJul 25 2025, 1:57 AM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".Jul 25 2025, 1:57 AM

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: secure-revision.

Harbormaster completed remote builds in B823084: Diff 1095719.Jul 25 2025, 2:25 AM

saschanaz planned changes to this revision.Jul 25 2025, 4:54 PM

saschanaz updated this revision to Diff 1096057.

Harbormaster completed remote builds in B823341: Diff 1096057.Jul 25 2025, 5:24 PM

saschanaz requested review of this revision.Jul 29 2025, 5:36 PM

saschanaz updated this revision to Diff 1097579.

saschanaz retitled this revision from WIP: Bug 1965029 - Accept CSS color syntax in `input type=color` to Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio.

saschanaz edited the summary of this revision. (Show Details)

saschanaz added a reviewer: emilio.

Herald added a subscriber: zrhoffman. · View Herald TranscriptJul 29 2025, 5:36 PM

saschanaz updated this revision to Diff 1097581.Jul 29 2025, 5:41 PM

saschanaz added inline comments.Jul 29 2025, 5:43 PM

dom/html/HTMLInputElement.cpp
795	(Keeping the serialization as a separate function as ultimately I want to pass the structured object to the color picker popup)

Harbormaster completed remote builds in B824575: Diff 1097579.Jul 29 2025, 5:57 PM

Harbormaster completed remote builds in B824625: Diff 1097581.Jul 29 2025, 6:01 PM

emilio requested changes to this revision.Jul 30 2025, 10:25 AM

emilio added inline comments.

dom/html/HTMLInputElement.cpp
741	Pass the styleset instead for consistency with `ComputeColor`?
792	Use `OwnerDoc()`. Also that can't return null so...
layout/style/ServoCSSParser.h
92	This should document that invalid colors return black in the specifiied color space. Also, document the `aToColorSpace` parameter?
94	Maybe just `ComputeAbsoluteColor`? Or link to the spec, because otherwise the name is quite bizarre.
servo/ports/geckolib/glue.rs
8451	Is it worth signaling the error somehow rather than returning black?

This revision now requires changes to proceed.Jul 30 2025, 10:25 AM

r? for questions around OwnerDoc and RawData.

dom/html/HTMLInputElement.cpp
741	It only needs RawData anyway? I'd rather change ComputeColor for consistency. Does that make sense?
792	But OwnerDoc() behaves differently with disconnected nodes, and we probably don't want that?
servo/ports/geckolib/glue.rs
8451	For now I don't see the reason why we would want the error signal. (It's not that there's an error event...)

saschanaz added inline comments.Jul 30 2025, 10:32 AM

dom/html/HTMLInputElement.cpp
792	Oh wait, it's not ComposedDoc. I'll do that.

saschanaz updated this revision to Diff 1098022.Jul 30 2025, 10:33 AM

emilio added inline comments.Jul 30 2025, 10:53 AM

dom/html/HTMLInputElement.cpp
741	It only needs RawData anyway? I'd rather change ComputeColor for consistency. Does that make sense? Sure

Harbormaster completed remote builds in B826621: Diff 1098022.Jul 30 2025, 11:03 AM

(Seems like a lot of comments aren't addressed)

This revision now requires changes to proceed.Jul 30 2025, 11:56 AM

saschanaz requested review of this revision.Jul 30 2025, 12:29 PM

saschanaz updated this revision to Diff 1098071.

saschanaz added inline comments.Jul 30 2025, 12:30 PM

dom/canvas/CanvasRenderingContext2D.cpp
1312	Should this also just do `document->EnsureStyleSet()->RawData()` instead of getting it from PresShell? 🤔

Some nits, but looks good with those.

dom/canvas/CanvasRenderingContext2D.cpp
1312	Should this also just do `document->EnsureStyleSet()->RawData()` instead of getting it from PresShell? 🤔 I'd prefer not to. The styleset without the presshell is not very useful fwiw.
layout/inspector/InspectorUtils.cpp
770	StyleSet is non-null, just `return ps->StyleSet()->RawData();`
servo/ports/geckolib/glue.rs
8411–8412	`let computed = specified::Color::parse_and_compute(...)?;`, now that we propagate the `Option<>`
8439	let Some(result) = compute_color(..) else { return false };

This revision is now accepted and ready to land.Jul 30 2025, 12:40 PM

Harbormaster completed remote builds in B826810: Diff 1098071.Jul 30 2025, 12:50 PM

saschanaz updated this revision to Diff 1098107.Jul 30 2025, 1:25 PM

Harbormaster completed remote builds in B826838: Diff 1098107.Jul 30 2025, 1:43 PM

saschanaz updated this revision to Diff 1098125.Jul 30 2025, 1:48 PM

saschanaz marked 7 inline comments as done.

saschanaz marked 3 inline comments as done.Jul 30 2025, 1:52 PM

Harbormaster completed remote builds in B826856: Diff 1098125.Jul 30 2025, 2:16 PM

Closed by commit rFIREFOXAUTOLANDf360400c2978: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio (authored by saschanaz). · Explain WhyJul 30 2025, 4:55 PM

This revision was automatically updated to reflect the committed changes.

saschanaz added a commit: rFIREFOXAUTOLANDf360400c2978: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio.

chorotan reopened this revision.Jul 30 2025, 6:24 PM

This revision is now accepted and ready to land.Jul 30 2025, 6:24 PM

chorotan added a reverting change: rFIREFOXAUTOLAND89e73abd8bfa: Revert "Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio"….Jul 30 2025, 6:24 PM

saschanaz updated this revision to Diff 1099084.Jul 31 2025, 5:19 PM

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 31 2025, 5:19 PM

Harbormaster completed remote builds in B828945: Diff 1099084.Jul 31 2025, 5:43 PM

Closed by commit rFIREFOXAUTOLANDbf673891ceb2: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio (authored by saschanaz). · Explain WhyJul 31 2025, 10:45 PM

This revision was automatically updated to reflect the committed changes.

saschanaz added a commit: rFIREFOXAUTOLANDbf673891ceb2: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio.

Harbormaster completed remote builds in B829372: Diff 1099312.Aug 1 2025, 3:26 PM

saschanaz mentioned this in rFIREFOXBETAf360400c2978: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio.Aug 18 2025, 1:42 PM

saschanaz mentioned this in rFIREFOXBETAbf673891ceb2: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio.Aug 18 2025, 1:45 PM

saschanaz mentioned this in rFIREFOXRELEASEf360400c2978: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio.Sep 8 2025, 1:59 PM

saschanaz mentioned this in rFIREFOXRELEASEbf673891ceb2: Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio.Sep 8 2025, 2:03 PM

Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 1099312

dom/canvas/CanvasRenderingContext2D.cpp

dom/html/HTMLInputElement.h

dom/html/HTMLInputElement.cpp

dom/html/test/forms/test_input_color_picker_datalist.html

dom/html/test/forms/test_input_sanitization.html

layout/inspector/InspectorUtils.cpp

layout/style/ServoCSSParser.h

layout/style/ServoCSSParser.cpp

servo/ports/geckolib/glue.rs

testing/web-platform/meta/html/semantics/forms/the-input-element/color.window.js.ini

testing/web-platform/tests/html/semantics/forms/the-input-element/color.html

testing/web-platform/tests/html/semantics/forms/the-input-element/color.tentative.html

testing/web-platform/tests/html/semantics/forms/the-input-element/color.window.js

Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilioClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 1099312

dom/canvas/CanvasRenderingContext2D.cpp

dom/html/HTMLInputElement.h

dom/html/HTMLInputElement.cpp

dom/html/test/forms/test_input_color_picker_datalist.html

dom/html/test/forms/test_input_sanitization.html

layout/inspector/InspectorUtils.cpp

layout/style/ServoCSSParser.h

layout/style/ServoCSSParser.cpp

servo/ports/geckolib/glue.rs

testing/web-platform/meta/html/semantics/forms/the-input-element/color.window.js.ini

testing/web-platform/tests/html/semantics/forms/the-input-element/color.html

testing/web-platform/tests/html/semantics/forms/the-input-element/color.tentative.html

testing/web-platform/tests/html/semantics/forms/the-input-element/color.window.js

Bug 1965029 - Accept CSS color syntax in `input type=color` r=emilio
ClosedPublic
Actions

Revision Contents
Changeset List