Bug 201739

Summary: getPropertyValue() and getComputedStyle() for text-rendering should return lower case string
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: CSSAssignee: 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: P2    
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

Description Said Abou-Hallawa 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.
Comment 2 Rob Buis 2021-09-17 09:12:05 PDT
Created attachment 438480 [details]
Patch
Comment 3 Rob Buis 2021-09-17 10:14:59 PDT
Created attachment 438487 [details]
Patch
Comment 4 Rob Buis 2021-09-17 11:53:13 PDT
Created attachment 438497 [details]
Patch
Comment 5 Darin Adler 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?
Comment 6 Rob Buis 2021-09-19 05:15:08 PDT
Created attachment 438590 [details]
Patch
Comment 7 Rob Buis 2021-09-19 06:39:31 PDT
Created attachment 438592 [details]
Patch
Comment 8 Rob Buis 2021-09-19 08:38:41 PDT
Created attachment 438595 [details]
Patch
Comment 9 Rob Buis 2021-09-19 11:04:33 PDT
Created attachment 438602 [details]
Patch
Comment 10 Rob Buis 2021-09-19 12:52:00 PDT
Created attachment 438612 [details]
Patch
Comment 11 Rob Buis 2021-09-19 23:01:36 PDT
Created attachment 438638 [details]
Patch
Comment 12 Rob Buis 2021-09-20 00:33:36 PDT
Created attachment 438642 [details]
Patch
Comment 13 Rob Buis 2021-09-20 07:41:22 PDT
Created attachment 438678 [details]
Patch
Comment 14 Tim Nguyen (:ntim) 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.
Comment 16 EWS 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.
Comment 18 EWS 2022-12-08 22:12:34 PST
rabbitboizaa@gmail.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/metadata/contributors.json.

Rejecting attachment 463956 [details] from commit queue.
Comment 19 Ahmad Saleem 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?
Comment 20 Anne van Kesteren 2023-12-24 02:49:53 PST
This was fixed in bug 249438 but I wonder if Tim's fix would've been better.
Comment 21 Anne van Kesteren 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 ***