Bug 239989

<rdar://problem/92992438>

Created attachment 459115 [details] Patch

Comment on attachment 459115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=459115&action=review I originally intentionally changed -webkit-text-combine to be hidden from the computed style when implementing text-combine-upright since it is non-standard. But on second thought, we should probably just handle this like any alias. > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3972 > + return cssValuePool.createValue(style.textCombine()); I wonder what we should do if we ever add the digits value support to text-combine-upright. How would that reflect in -webkit-text-combine?

(In reply to Tim Nguyen (:ntim) from comment #3) > Comment on attachment 459115 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=459115&action=review > > I originally intentionally changed -webkit-text-combine to be hidden from > the computed style when implementing text-combine-upright since it is > non-standard. But it's still indexed, so it's weird. Another alternative could be not indexing it, though. > But on second thought, we should probably just handle this like any alias. Aliases have the same grammar, which is not the case here and changing it might have a compat risk. I think the proper way would be as a legacy shorthand: https://drafts.csswg.org/css-cascade-4/#legacy-shorthand > > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3972 > > + return cssValuePool.createValue(style.textCombine()); > > I wonder what we should do if we ever add the digits value support to > text-combine-upright. How would that reflect in -webkit-text-combine? I guess it depends on whether the grammar -webkit-text-combine is also expanded to accept that or not. If -webkit-text-combine accepts digits, then it should serialize normally. If it doesn't, then empty string.

(In reply to Oriol Brufau from comment #4) > (In reply to Tim Nguyen (:ntim) from comment #3) > > Comment on attachment 459115 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=459115&action=review > > > > I originally intentionally changed -webkit-text-combine to be hidden from > > the computed style when implementing text-combine-upright since it is > > non-standard. > > But it's still indexed, so it's weird. Another alternative could be not > indexing it, though. > > > But on second thought, we should probably just handle this like any alias. > > Aliases have the same grammar, which is not the case here and changing it > might have a compat risk. > I think the proper way would be as a legacy shorthand: > https://drafts.csswg.org/css-cascade-4/#legacy-shorthand Legacy shorthand sounds good. > > > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:3972 > > > + return cssValuePool.createValue(style.textCombine()); > > > > I wonder what we should do if we ever add the digits value support to > > text-combine-upright. How would that reflect in -webkit-text-combine? > > I guess it depends on whether the grammar -webkit-text-combine is also > expanded to accept that or not. > If -webkit-text-combine accepts digits, then it should serialize normally. > If it doesn't, then empty string. -webkit-text-combine should not accept any new syntax, so probably empty string makes sense or not indexing. Anyway, no need to think about this now, but worth leaving a note.

(In reply to Tim Nguyen (:ntim) from comment #5) > Legacy shorthand sounds good. Filed bug 240341.

Committed r294101 (250486@main): <https://commits.webkit.org/250486@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 459115 [details].