RESOLVED FIXED 211672
CSS Variables: Color on specific `border` properties does not work. 
https://bugs.webkit.org/show_bug.cgi?id=211672
Summary CSS Variables: Color on specific `border` properties does not work.
ed7-aspire4925
Reported 2020-05-09 13:27:27 PDT
Created attachment 398937 [details] sample Color CSS variables set in the following `border` properties do not work: - border-block-start - border-block-end - border-inline-start - border-inline-end See attachment or [https://jsfiddle.net/f2u4w9g6/1] WebKitGTK 2.28.2-2~deb10u1 Tested on Midori and GNOME Web, both use the same engine.
Attachments
sample (646 bytes, text/html)
2020-05-09 13:27 PDT, ed7-aspire4925 		no flags
Details
Patch (16.83 KB, patch)
2020-06-03 22:15 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (6.44 KB, patch)
2020-06-04 16:21 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (6.42 KB, patch)
2020-06-04 16:27 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (6.90 KB, patch)
2020-06-04 17:04 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (6.89 KB, patch)
2020-06-04 17:22 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (6.15 KB, patch)
2020-06-04 20:26 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (6.19 KB, patch)
2020-06-04 21:13 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Patch (6.19 KB, patch)
2020-06-04 21:32 PDT, Tyler Wilcock 		no flags
DetailsFormatted DiffDiff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-05-12 14:07:53 PDT
<rdar://problem/63153128>
Tyler Wilcock
Comment 2 2020-06-03 22:15:01 PDT
Created attachment 400997 [details] Patch
Simon Fraser (smfr)
Comment 3 2020-06-04 11:02:29 PDT
Comment on attachment 400997 [details] Patch Patch looks OK,but can you make a reference test (foo.html, foo-expected.html that render the same way). We don't add new -expected.png files any more.
Tyler Wilcock
Comment 4 2020-06-04 16:21:23 PDT
Created attachment 401089 [details] Patch
Darin Adler
Comment 5 2020-06-04 16:23:33 PDT
Comment on attachment 401089 [details] Patch Need to create a logical-border-props-with-variables-expected.html file that generates the same appearance, but without using variables. That will make the test a "reftest". Should not have an expected.txt or an expected.png.
Tyler Wilcock
Comment 6 2020-06-04 16:27:08 PDT
Created attachment 401091 [details] Patch
Tyler Wilcock
Comment 7 2020-06-04 16:29:13 PDT
Oops, thanks Darin, misread Simon's comment. Will do that now. I removed the review request on this latest patch.
Tyler Wilcock
Comment 8 2020-06-04 17:04:50 PDT
Created attachment 401096 [details] Patch
Tyler Wilcock
Comment 9 2020-06-04 17:22:23 PDT
Created attachment 401101 [details] Patch
Simon Fraser (smfr)
Comment 10 2020-06-04 19:59:27 PDT
Comment on attachment 401101 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401101&action=review > LayoutTests/fast/borders/logical-border-props-with-variables-expected.html:2 > +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> > +<html xmlns="http://www.w3.org/1999/xhtml"> Don't need XHTML here. This can just be <!DOCTYPE html><html> > LayoutTests/fast/borders/logical-border-props-with-variables-expected.html:8 > + <title> > + Test for https://bugs.webkit.org/show_bug.cgi?id=211672. The intention of this test is to ensure variables are > + usable as values in the `border-block-start`, `border-block-end`, `border-inline-start`, and `border-inline-end` > + properties. > + </title> I would not bother with this in the expected result. > LayoutTests/fast/borders/logical-border-props-with-variables-expected.html:11 > + <style type="text/css"> Just <style> > LayoutTests/fast/borders/logical-border-props-with-variables-expected.html:22 > + Test for https://bugs.webkit.org/show_bug.cgi?id=211672. Passes if there is a 3px solid green border around all four sides of this text. I would remove this text. Having visible text in a test makes it more likely to fail in future because of small antialiasing differences. > LayoutTests/fast/borders/logical-border-props-with-variables.html:25 > + Test for https://bugs.webkit.org/show_bug.cgi?id=211672. Passes if there is a 3px solid green border around all four sides of this text. Same comments apply to this file.
Tyler Wilcock
Comment 11 2020-06-04 20:26:51 PDT
Created attachment 401114 [details] Patch
Simon Fraser (smfr)
Comment 12 2020-06-04 20:59:29 PDT
Comment on attachment 401114 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401114&action=review > LayoutTests/fast/borders/logical-border-props-with-variables-expected.html:2 > +<html xmlns="http://www.w3.org/1999/xhtml"> No xmlns. > LayoutTests/fast/borders/logical-border-props-with-variables.html:25 > + <div> > + </div> I guess this now ends up as a 6x6px div. I think it would be better to give it a size (like 100x100px) and make the border super obvious (50px width). Don't be shy making things big!
Tyler Wilcock
Comment 13 2020-06-04 21:13:31 PDT
Created attachment 401116 [details] Patch
Simon Fraser (smfr)
Comment 14 2020-06-04 21:26:06 PDT
Comment on attachment 401116 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=401116&action=review > LayoutTests/ChangeLog:8 > + Add test verifying CSS variables work as values in the `border-block-start`, `border-block-end`, `border-inline-start`, and `border-inline-end` properties. > + > + Reviewed by NOBODY (OOPS!). The Reviewed by should go above, but that's fine.
EWS
Comment 15 2020-06-04 21:26:32 PDT
ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.
Tyler Wilcock
Comment 16 2020-06-04 21:32:18 PDT
Created attachment 401118 [details] Patch
Tyler Wilcock
Comment 17 2020-06-04 21:34:12 PDT
I added a new patch with the "Reviewed by" in the correct spot. Should this line be removed, or will EWS accept it now that it's in the right place?
EWS
Comment 18 2020-06-05 10:22:11 PDT
Committed r262627: <https://trac.webkit.org/changeset/262627> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401118 [details].
Tyler Wilcock
Comment 19 2020-06-05 10:33:49 PDT
Thanks for sticking with me on this one -- learned a lot!
Note You need to log in before you can comment on or make changes to this bug.