In the latest CSS Color 4 draft <https://drafts.csswg.org/css-color/>, the following are true: rgb() and rgba() are aliases. hls() and hlsa() are aliases. rgb() can take percentage or floating-point color components rgb() components do not have to be separated with commas. A slash can separate the r,g,b from the alpha component. The alpha component can be a percentage. Similar behaviors exist for the hsl() function. Web inspector needs to be able to parse these new formats.
<rdar://problem/47062403>
Created attachment 358761 [details] Patch
Comment on attachment 358761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358761&action=review > LayoutTests/inspector/model/color.html:70 > + testGood("rgb(1 2 3)", WI.Color.Format.RGB); You need to test percentage rgb components, and decimal values. See https://drafts.csswg.org/css-color/#rgb-functions for the allowed syntax. Also you should test a mix of percentages and non-percentages, with and without the slash syntax.
Comment on attachment 358761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358761&action=review >> LayoutTests/inspector/model/color.html:70 >> + testGood("rgb(1 2 3)", WI.Color.Format.RGB); > > You need to test percentage rgb components, and decimal values. See https://drafts.csswg.org/css-color/#rgb-functions for the allowed syntax. > > Also you should test a mix of percentages and non-percentages, with and without the slash syntax. Crap! I added support for that, but I forgot to test it 😅 Reading that document, it also looks like `deg` is supported with the hue of `hsl()` functions, so we should support that too.
Comment on attachment 358761 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358761&action=review >>> LayoutTests/inspector/model/color.html:70 >>> + testGood("rgb(1 2 3)", WI.Color.Format.RGB); >> >> You need to test percentage rgb components, and decimal values. See https://drafts.csswg.org/css-color/#rgb-functions for the allowed syntax. >> >> Also you should test a mix of percentages and non-percentages, with and without the slash syntax. > > Crap! I added support for that, but I forgot to test it 😅 > > Reading that document, it also looks like `deg` is supported with the hue of `hsl()` functions, so we should support that too. Not just deg. Any angle unit.
Created attachment 358767 [details] Patch
Comment on attachment 358767 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358767&action=review > LayoutTests/inspector/model/color.html:236 > + testBad("rgb(255, 255, 255, 0.5, 1)"); // extra values I think you need more here. Things with a mix of % and number. Things with both commas and slash.
Comment on attachment 358767 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358767&action=review >> LayoutTests/inspector/model/color.html:236 >> + testBad("rgb(255, 255, 255, 0.5, 1)"); // extra values > > I think you need more here. Things with a mix of % and number. Things with both commas and slash. WebInspector actually has traditionally been very lenient with "invalid" values (e.g. you'll still get a swatch for `color: hsl(10, 20, 30);` even though it's missing "%" for saturation/lightness). I'm of the opinion that we should continue this tradition. The regular expressions I've used here are pretty lenient as to whether it actually is parseable, or whether it "looks" parseable. Mixing commas/slashes or %/number/<other unit> is somewhat irrelevant as to whether WebInspector can recognize what you've written as a color value. I don't think we want to re-implement the CSS parser in the WebInspector frontend. Perhaps we can add a button to the swatch UI that says something like "we figured out what color you were trying to display, but you didn't write it correctly, so click [here] to print what it should be" (e.g. it would replace `hsl(10, 20, 30)` with `hsl(10, 20%, 30%)`).
Comment on attachment 358767 [details] Patch Rejecting attachment 358767 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 358767, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=358767&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=193166&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Updating working directory Processing patch 358767 from bug 193166. Fetching: https://bugs.webkit.org/attachment.cgi?id=358767 Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date W: fb5ce102a854a5fbe3ed20114179a623a23e671c and refs/remotes/origin/master differ, using rebase: :040000 040000 5c6fa086a506eaf3ff0e8a1b325a94fbc5dce80d 588baa2b22fc6923312211f40013fe0e4f6a6647 M LayoutTests :040000 040000 ac7862ab69fef5d2bb84a4baa38727b049620324 14d551313a92449e3f18ff06fffe5925efe69e3b M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog ERROR from SVN: Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date W: fb5ce102a854a5fbe3ed20114179a623a23e671c and refs/remotes/origin/master differ, using rebase: :040000 040000 5c6fa086a506eaf3ff0e8a1b325a94fbc5dce80d 588baa2b22fc6923312211f40013fe0e4f6a6647 M LayoutTests :040000 040000 ac7862ab69fef5d2bb84a4baa38727b049620324 14d551313a92449e3f18ff06fffe5925efe69e3b M Source Current branch master is up to date. ERROR: Not all changes have been committed into SVN, however the committed ones (if any) seem to be successfully integrated into the working tree. Please see the above messages for details. Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Updating OpenSource Current branch master is up to date. Full output: https://webkit-queues.webkit.org/results/10904419
Created attachment 360252 [details] Patch
Comment on attachment 360252 [details] Patch Clearing flags on attachment: 360252 Committed r240550: <https://trac.webkit.org/changeset/240550>
All reviewed patches have been landed. Closing bug.
This doesn't update `createCodeMirrorColorTextMarkers` in CodeMirrorTextMarkers.js. That means that we won't get inline editors for colors with the new syntax in CSS resources.
(In reply to Joseph Pecoraro from comment #13) > This doesn't update `createCodeMirrorColorTextMarkers` in > CodeMirrorTextMarkers.js. That means that we won't get inline editors for > colors with the new syntax in CSS resources. The regex in `createCodeMirrorColorTextMarkers` is able to handle these syntaxes. It's much more general, and really relies more on `WI.Color.fromString` to do the parsing.
(In reply to Devin Rousso from comment #14) > (In reply to Joseph Pecoraro from comment #13) > > This doesn't update `createCodeMirrorColorTextMarkers` in > > CodeMirrorTextMarkers.js. That means that we won't get inline editors for > > colors with the new syntax in CSS resources. > The regex in `createCodeMirrorColorTextMarkers` is able to handle these > syntaxes. It's much more general, and really relies more on > `WI.Color.fromString` to do the parsing. Interesting, you're right. I didn't realize it was so generic.