Bug 201739

Summary:

getPropertyValue() and getComputedStyle() for text-rendering should return lower case string

Product:

WebKit

Reporter:

Said Abou-Hallawa <sabouhallawa>

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED DUPLICATE

Severity:

Normal

CC:

ahmad.saleem792, annevk, darin, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, koivisto, macpherson, menard, ntim, rabbitboizaa, rbuis, zimmermann

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

200143

Attachments:

Description	Flags
text-rendering	none
Patch	none
Patch	none
Patch	none
Patch	ews-feeder: commit-queue-
Patch	none
Patch	ews-feeder: commit-queue-
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none

Said Abou-Hallawa

Reported 2019-09-12 13:36:32 PDT

Created attachment 378674 [details] text-rendering Open the attached test case. Result: The computed style values for text-rendering are in camel case. Expected: Like all the other computed style values, they should be in lower case.

Attachments
text-rendering (1.20 KB, image/svg+xml) 2019-09-12 13:36 PDT, Said Abou-Hallawa	no flags	Details
Patch (5.96 KB, patch) 2021-09-17 09:12 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (10.71 KB, patch) 2021-09-17 10:14 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (10.06 KB, patch) 2021-09-17 11:53 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (10.01 KB, patch) 2021-09-19 05:15 PDT, Rob Buis	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (2.67 KB, patch) 2021-09-19 06:39 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (4.04 KB, patch) 2021-09-19 08:38 PDT, Rob Buis	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch (67.86 KB, patch) 2021-09-19 11:04 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (78.57 KB, patch) 2021-09-19 12:52 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (79.16 KB, patch) 2021-09-19 23:01 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (79.74 KB, patch) 2021-09-20 00:33 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Patch (141.79 KB, patch) 2021-09-20 07:41 PDT, Rob Buis	no flags	Details Formatted Diff Diff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Said Abou-Hallawa

Comment 1 2019-09-12 13:42:23 PDT

Affected WPT tests are: https://wpt.fyi/results/svg/painting/parsing/text-rendering-valid.svg?label=master&label=experimental&aligned https://wpt.fyi/results/svg/painting/parsing/text-rendering-computed.svg?label=master&label=experimental&aligned

Rob Buis

Comment 2 2021-09-17 09:12:05 PDT

Created attachment 438480 [details] Patch

Rob Buis

Comment 3 2021-09-17 10:14:59 PDT

Created attachment 438487 [details] Patch

Rob Buis

Comment 4 2021-09-17 11:53:13 PDT

Created attachment 438497 [details] Patch

Darin Adler

Comment 5 2021-09-18 15:57:58 PDT

Comment on attachment 438497 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=438497&action=review > Source/WebCore/ChangeLog:9 > + Explicitly lowercase computed style values for the text-rendering property > + since it uses camelcase for some of its values. This does not look like the right way to achieve this. Why do we "use camelcase" for some of these values if that is not what is expected? That is what we should fix. We shouldn’t be converting to lowercase at the boundary like this. Unless there is some context where the camel case values are needed? Does the standard actually require the camel case when specifying values and lowercase when inspecting them?

Rob Buis

Comment 6 2021-09-19 05:15:08 PDT

Created attachment 438590 [details] Patch

Rob Buis

Comment 7 2021-09-19 06:39:31 PDT

Created attachment 438592 [details] Patch

Rob Buis

Comment 8 2021-09-19 08:38:41 PDT

Created attachment 438595 [details] Patch

Rob Buis

Comment 9 2021-09-19 11:04:33 PDT

Created attachment 438602 [details] Patch

Rob Buis

Comment 10 2021-09-19 12:52:00 PDT

Created attachment 438612 [details] Patch

Rob Buis

Comment 11 2021-09-19 23:01:36 PDT

Created attachment 438638 [details] Patch

Rob Buis

Comment 12 2021-09-20 00:33:36 PDT

Created attachment 438642 [details] Patch

Rob Buis

Comment 13 2021-09-20 07:41:22 PDT

Created attachment 438678 [details] Patch

Tim Nguyen (:ntim)

Comment 14 2022-02-26 02:43:52 PST

Shouldn't we change the values to lowercase directly in https://webkit-search.igalia.com/webkit/rev/820cbd8df3d5043b61b3a8142e1df68b3efc3f6b/Source/WebCore/css/CSSValueKeywords.in#1080-1082 ? +CC Antti who probably is a good person to look at this patch.

EWS

Comment 16 2022-12-08 20:02:15 PST

rabbitboizaa@gmail.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/metadata/contributors.json. Rejecting attachment 463951 [details] from commit queue.

EWS

Comment 18 2022-12-08 22:12:34 PST

Ahmad Saleem

Comment 19 2023-04-13 15:59:54 PDT

(In reply to Said Abou-Hallawa from comment #1) > Affected WPT tests are: > > https://wpt.fyi/results/svg/painting/parsing/text-rendering-valid. > svg?label=master&label=experimental&aligned > https://wpt.fyi/results/svg/painting/parsing/text-rendering-computed. > svg?label=master&label=experimental&aligned Safari Technology Preview 167 is passing these tests, do we need this anymore?

Anne van Kesteren

Comment 20 2023-12-24 02:49:53 PST

This was fixed in bug 249438 but I wonder if Tim's fix would've been better.

Anne van Kesteren

Comment 21 2023-12-24 03:56:56 PST

Unfortunately lowercasing at source has a number of implications for all kinds of enums. It's probably the correct idea, but it would require some work and probably a little more insight into how it all fits together than I have right now. I filed bug 266863 to investigate this more. *** This bug has been marked as a duplicate of bug 249438 ***

Note You need to log in before you can comment on or make changes to this bug.