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: | P2 | ||||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||||||
Bug Blocks: | 200143 | ||||||||||||||||||||||||||||
Attachments: |
|
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 Created attachment 438480 [details]
Patch
Created attachment 438487 [details]
Patch
Created attachment 438497 [details]
Patch
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? Created attachment 438590 [details]
Patch
Created attachment 438592 [details]
Patch
Created attachment 438595 [details]
Patch
Created attachment 438602 [details]
Patch
Created attachment 438612 [details]
Patch
Created attachment 438638 [details]
Patch
Created attachment 438642 [details]
Patch
Created attachment 438678 [details]
Patch
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. 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. 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. (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? This was fixed in bug 249438 but I wonder if Tim's fix would've been better. 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 *** |
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.