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.
<rdar://problem/63153128>
Created attachment 400997 [details] Patch
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.
Created attachment 401089 [details] Patch
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.
Created attachment 401091 [details] Patch
Oops, thanks Darin, misread Simon's comment. Will do that now. I removed the review request on this latest patch.
Created attachment 401096 [details] Patch
Created attachment 401101 [details] Patch
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.
Created attachment 401114 [details] Patch
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!
Created attachment 401116 [details] Patch
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.
ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.
Created attachment 401118 [details] Patch
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?
Committed r262627: <https://trac.webkit.org/changeset/262627> All reviewed patches have been landed. Closing bug and clearing flags on attachment 401118 [details].
Thanks for sticking with me on this one -- learned a lot!