| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
I think this largely looks good to me, with a few comments below.
I did a decent amount of spec reading (and made [one spec PR](https://github.com/whatwg/html/pull/12606)!) although I don't think I reviewed all of the relevant pieces of spec. But hopefully the tests have good enough coverage!
ChromeStatus: https://chromestatus.com/feature/5736024861966336Side comment: It might be good to give this chromestatus entry a more descriptive name. (`InputTypeColorEnhancements` should still be the finch feature name.)
Also, this `Chromestatus:` line could be a proper git footer if you want, adjacent to `Bug:` and `Change-Id:`.
// attributes, per the value sanitization algorithm for type=color.More directly I think this is https://html.spec.whatwg.org/multipage/input.html#serialize-a-color-well-control-color which is just a part of the value sanitization algorithm.
bool AlphaAttributeEnabled() const;I'm a bit worried `AlphaAttributeEnabled` sounds too much like a test for whether a feature is enabled. Maybe something more like `HasAlphaComponent` would be better?
AddWarningToConsole(
"The specified value %s does not conform to the required format. The "
"format is \"#rrggbb\" where rr, gg, bb are two-digit hexadecimal "
"numbers.",I think this warning could use some updates?
GetElement().SetNonDirtyValue(I didn't see a test for this case of reparsing the original value rather than the current one. If there indeed isn't one, could you add one? (It could cite https://github.com/whatwg/html/issues/12057 .)
if (RuntimeEnabledFeatures::InputTypeColorEnhancementsEnabled()) {I think this `RuntimeEnabledFeatures` check belongs better inside `ColorInputType::ColorSpaceOrAlphaAttributeChanged`
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Are you planning to make additional changes to the color picker to allow the user to choose alpha?
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Will address reviews later today.
Are you planning to make additional changes to the color picker to allow the user to choose alpha?
I am planning to do it in the following CLs, so the answer is yes :)
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
ChromeStatus: https://chromestatus.com/feature/5736024861966336Side comment: It might be good to give this chromestatus entry a more descriptive name. (`InputTypeColorEnhancements` should still be the finch feature name.)
Also, this `Chromestatus:` line could be a proper git footer if you want, adjacent to `Bug:` and `Change-Id:`.
Done
// attributes, per the value sanitization algorithm for type=color.More directly I think this is https://html.spec.whatwg.org/multipage/input.html#serialize-a-color-well-control-color which is just a part of the value sanitization algorithm.
Done
I'm a bit worried `AlphaAttributeEnabled` sounds too much like a test for whether a feature is enabled. Maybe something more like `HasAlphaComponent` would be better?
Done
AddWarningToConsole(
"The specified value %s does not conform to the required format. The "
"format is \"#rrggbb\" where rr, gg, bb are two-digit hexadecimal "
"numbers.",I think this warning could use some updates?
Done
GetElement().SetNonDirtyValue(Jason LeoI didn't see a test for this case of reparsing the original value rather than the current one. If there indeed isn't one, could you add one? (It could cite https://github.com/whatwg/html/issues/12057 .)
added one
if (RuntimeEnabledFeatures::InputTypeColorEnhancementsEnabled()) {I think this `RuntimeEnabledFeatures` check belongs better inside `ColorInputType::ColorSpaceOrAlphaAttributeChanged`
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/60837.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Commit-Queue | +2 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
9 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/web_tests/external/wpt/html/dom/idlharness.https_include=HTML.+-expected.txt
Insertions: 1, Deletions: 3.
The diff is too large to show. Please review the diff.
```
Support colorspace and alpha attributes on <input type=color>
Adds the colorspace/colorSpace[1] and alpha[2] content/IDL attributes
behind the experimental InputTypeColorEnhancements flag, letting color
inputs serialize wide-gamut (Display P3) and alpha-bearing values per
the HTML spec. Achromatic sRGB colors are converted to Display P3
directly to avoid float-matrix white-point drift.
[1]: https://html.spec.whatwg.org/multipage/input.html#dom-input-colorspace
[2]: https://html.spec.whatwg.org/multipage/input.html#attr-input-alpha
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
The exported PR, https://github.com/web-platform-tests/wpt/pull/60837, has failed the following check(s) on GitHub:
lint (https://github.com/web-platform-tests/wpt/runs/83278786743)
These failures will block the export. They may represent new or existing problems; please take a look at the output and see if it can be fixed. Unresolved failures will be looked at by the Ecosystem-Infra sheriff after this CL has been landed in Chromium; if you need earlier help please contact blin...@chromium.org.
Any suggestions to improve this service are welcome; crbug.com/1027618.
Gerrit CL SHA: Latest
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/60837