Bug 211672

Summary:

CSS Variables: Color on specific `border` properties does not work.

Product:

WebKit

Reporter:

ed7-aspire4925

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, koivisto, macpherson, menard, simon.fraser, twilco.o, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

Other

OS:

Other

Attachments:

Description	Flags
sample	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

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	Details Formatted Diff Diff
Patch (6.44 KB, patch) 2020-06-04 16:21 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (6.42 KB, patch) 2020-06-04 16:27 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (6.90 KB, patch) 2020-06-04 17:04 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (6.89 KB, patch) 2020-06-04 17:22 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (6.15 KB, patch) 2020-06-04 20:26 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (6.19 KB, patch) 2020-06-04 21:13 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
Patch (6.19 KB, patch) 2020-06-04 21:32 PDT, Tyler Wilcock	no flags	Details Formatted Diff Diff
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.