Bug 232729

Summary: Rename TextDecoration to TextDecorationLine
Product: WebKit Reporter: Nikos Mouchtaris <nmouchtaris>
Component: CSSAssignee: Nikos Mouchtaris <nmouchtaris>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, changseok, dino, dmazzoni, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, jcraig, jdiggs, kondapallykalyan, macpherson, menard, mifenton, mmaxfield, ntim, pdr, sabouhallawa, samuel_white, schenney, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 230083    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
mmaxfield: review+
Patch
none
Patch none

Nikos Mouchtaris
Reported 2021-11-04 15:30:25 PDT
Rename TextDecoration to TextDecorationLine
Attachments
Patch (57.70 KB, patch)
2021-11-04 16:32 PDT, Nikos Mouchtaris 		no flags
DetailsFormatted DiffDiff
Patch (54.45 KB, patch)
2021-11-04 18:05 PDT, Nikos Mouchtaris 		no flags
DetailsFormatted DiffDiff
Patch (56.06 KB, patch)
2021-11-05 12:36 PDT, Nikos Mouchtaris 		mmaxfield: review+
DetailsFormatted DiffDiff
Patch (50.74 KB, patch)
2021-11-16 12:13 PST, Nikos Mouchtaris 		no flags
DetailsFormatted DiffDiff
Patch (50.75 KB, patch)
2021-11-16 16:31 PST, Nikos Mouchtaris 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Nikos Mouchtaris
Comment 1 2021-11-04 16:32:13 PDT
Created attachment 443354 [details] Patch
Nikos Mouchtaris
Comment 2 2021-11-04 18:05:44 PDT
Created attachment 443365 [details] Patch
Nikos Mouchtaris
Comment 3 2021-11-05 12:36:42 PDT
Created attachment 443424 [details] Patch
Myles C. Maxfield
Comment 4 2021-11-05 16:57:22 PDT
Why is there no change to CSSProperties.json?
Myles C. Maxfield
Comment 5 2021-11-05 17:01:46 PDT
(In reply to Myles C. Maxfield from comment #4) > Why is there no change to CSSProperties.json? Oh, text-decoration-line already appears in CSSProperties.json. It looks like we have text-decoration, which has `"converter": "TextDecoration"`, and we also have -webkit-text-decoration, which is a longhand for text-decoration-line (and 2 others) 🤔
Myles C. Maxfield
Comment 6 2021-11-05 17:06:39 PDT
Neither this bug, nor the changelog in this patch, nor the blocked bug (https://bugs.webkit.org/show_bug.cgi?id=230083) describe how this patch fits into the overall strategy for how we're going to achieve https://bugs.webkit.org/show_bug.cgi?id=230083. It's difficult to review this without understanding how this patch fits into an overall strategy.
Tim Nguyen (:ntim)
Comment 7 2021-11-06 09:53:26 PDT
(In reply to Myles C. Maxfield from comment #6) > Neither this bug, nor the changelog in this patch, nor the blocked bug > (https://bugs.webkit.org/show_bug.cgi?id=230083) describe how this patch > fits into the overall strategy for how we're going to achieve > https://bugs.webkit.org/show_bug.cgi?id=230083. It's difficult to review > this without understanding how this patch fits into an overall strategy. text-decoration right now is implemented as a keyword property: underline/overline/strikethrough/etc. The spec says those keywords should be for the text-decoration-line property, and text-decoration should be a shorthand for text-decoration-line/color/style/thickness. Renaming the TextDecoration enum to TextDecorationLine reduces ambiguity.
Radar WebKit Bug Importer
Comment 8 2021-11-11 14:31:21 PST
<rdar://problem/85315983>
Tim Nguyen (:ntim)
Comment 9 2021-11-15 15:09:54 PST
Comment on attachment 443424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443424&action=review > Source/WebCore/rendering/style/StyleVisualData.cpp:31 > + , textDecorationLine(RenderStyle::initialTextDecoration().toRaw()) Since this is linked to the property name, rather than the value type, I would rename this specific field in the other bug.
Myles C. Maxfield
Comment 10 2021-11-15 19:56:11 PST
Comment on attachment 443424 [details] Patch Oh, this is just the enum. I guess that's fine.
Nikos Mouchtaris
Comment 11 2021-11-16 12:13:26 PST
Created attachment 444420 [details] Patch
Nikos Mouchtaris
Comment 12 2021-11-16 16:31:51 PST
Created attachment 444448 [details] Patch
EWS
Comment 13 2021-11-16 17:27:05 PST
Committed r285904 (244317@main): <https://commits.webkit.org/244317@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 444448 [details].
Note You need to log in before you can comment on or make changes to this bug.