239989 2022-05-02 16:20:23 -0700 [cssom] -webkit-text-combine:none serializes as empty string in computed style 2022-05-12 06:50:23 -0700 1 1 1 Unclassified WebKit CSS WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 obrufau obrufau esprehn+autocc ews-watchlist glenn gyuyoung.kim macpherson menard ntim webkit-bug-importer oldest_to_newest 1865991 0 obrufau 2022-05-02 16:20:23 -0700 ```js var el = document.documentElement; el.style.webkitTextCombine = "horizontal"; el.style.webkitTextCombine; // "horizontal" getComputedStyle(el).webkitTextCombine; // "horizontal" el.style.webkitTextCombine = "none"; el.style.webkitTextCombine; // "none" getComputedStyle(el).webkitTextCombine; // "" ``` The last line should produce "none". 1867804 1 webkit-bug-importer 2022-05-09 16:21:12 -0700 <rdar://problem/92992438> 1867912 2 459115 obrufau 2022-05-10 06:11:01 -0700 Created attachment 459115 Patch 1867990 3 459115 ntim 2022-05-10 12:03:54 -0700 Comment on attachment 459115 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? 1867992 4 obrufau 2022-05-10 12:15:42 -0700 (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. 1868061 5 ntim 2022-05-10 23:24:16 -0700 (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. 1868374 6 obrufau 2022-05-12 06:24:39 -0700 (In reply to Tim Nguyen (:ntim) from comment #5) > Legacy shorthand sounds good. Filed bug 240341. 1868383 7 ews-feeder 2022-05-12 06:50:19 -0700 Committed r294101 (250486@main): <https://commits.webkit.org/250486@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 459115. 459115 2022-05-10 06:11:01 -0700 2022-05-12 06:50:21 -0700 Patch bug-239989-20220510151100.patch text/plain 3435 obrufau U3VidmVyc2lvbiBSZXZpc2lvbjogMjk0MDA2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYzI2MjFjMmQxOGQxOGNi OTA1MjQ0YmMyZDc4M2E3MzFjMTg0NDcwMy4uOWY3MjdlOGIwNGE5N2YwYWE0MDM0ZDYxZWQ0YzRj OTU1MGIxMjYzZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDIyLTA1LTEwICBPcmlv bCBCcnVmYXUgIDxvYnJ1ZmF1QGlnYWxpYS5jb20+CisKKyAgICAgICAgW2Nzc29tXSBTZXJpYWxp emUgY29tcHV0ZWQgJy13ZWJraXQtdGV4dC1jb21iaW5lOiBub25lJworICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjM5OTg5CisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhlIGNvbXB1dGVkIHZhbHVlIG9mICct d2Via2l0LXRleHQtY29tYmluZScgd2FzIG9ubHkgc2VyaWFsaXplZCBmb3IKKyAgICAgICAgJy13 ZWJraXQtdGV4dC1jb21iaW5lOiBob3Jpem9udGFsJyBvciAndGV4dC1jb21iaW5lLXVwcmlnaHQ6 IGFsbCcuCisKKyAgICAgICAgQnV0IHRoZSBpbml0aWFsIHZhbHVlICdub25lJyBqdXN0IHByb2R1 Y2VkIGFuIGVtcHR5IHN0cmluZy4KKworICAgICAgICBUZXN0OiBpbXBvcnRlZC93M2Mvd2ViLXBs YXRmb3JtLXRlc3RzL2Nzcy9jc3NvbS9zZXJpYWxpemUtYWxsLWxvbmdoYW5kcy5odG1sCisKKyAg ICAgICAgKiBjc3MvQ1NTQ29tcHV0ZWRTdHlsZURlY2xhcmF0aW9uLmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OkNvbXB1dGVkU3R5bGVFeHRyYWN0b3I6OnZhbHVlRm9yUHJvcGVydHlJblN0eWxlKToK KwogMjAyMi0wNS0xMCAgWW91ZW5uIEZhYmxldCAgPHlvdWVubkBhcHBsZS5jb20+CiAKICAgICAg ICAgQ3JlYXRlIGEgc3RvcHBlZCBDYWNoZVN0b3JhZ2VDb25uZWN0aW9uIGZvciB3b3JrZXJzIGlu IGNhc2Ugb2YgZGV0YWNoZWQgZG9jdW1lbnRzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9j c3MvQ1NTQ29tcHV0ZWRTdHlsZURlY2xhcmF0aW9uLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9D U1NDb21wdXRlZFN0eWxlRGVjbGFyYXRpb24uY3BwCmluZGV4IDhjZWI1ZDJlYTE3YTVkODU4ZDJi YWY0NDk5NWVhZDhiNmQ3MjRlZGQuLjA1MDAzMDA5NWUyZDJmY2RhNDQ5YWE2ZTUyM2NiZDU0NzZj NGVkNjkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NDb21wdXRlZFN0eWxlRGVj bGFyYXRpb24uY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NDb21wdXRlZFN0eWxlRGVj bGFyYXRpb24uY3BwCkBAIC0zOTY5LDcgKzM5NjksNyBAQCBSZWZQdHI8Q1NTVmFsdWU+IENvbXB1 dGVkU3R5bGVFeHRyYWN0b3I6OnZhbHVlRm9yUHJvcGVydHlJblN0eWxlKGNvbnN0IFJlbmRlclN0 eQogICAgICAgICBjYXNlIENTU1Byb3BlcnR5V2Via2l0VGV4dENvbWJpbmU6CiAgICAgICAgICAg ICBpZiAoc3R5bGUudGV4dENvbWJpbmUoKSA9PSBUZXh0Q29tYmluZTo6QWxsKQogICAgICAgICAg ICAgICAgIHJldHVybiBDU1NQcmltaXRpdmVWYWx1ZTo6Y3JlYXRlSWRlbnRpZmllcihDU1NWYWx1 ZUhvcml6b250YWwpOwotICAgICAgICAgICAgcmV0dXJuIG51bGxwdHI7CisgICAgICAgICAgICBy ZXR1cm4gY3NzVmFsdWVQb29sLmNyZWF0ZVZhbHVlKHN0eWxlLnRleHRDb21iaW5lKCkpOwogICAg ICAgICBjYXNlIENTU1Byb3BlcnR5VGV4dENvbWJpbmVVcHJpZ2h0OgogICAgICAgICAgICAgcmV0 dXJuIGNzc1ZhbHVlUG9vbC5jcmVhdGVWYWx1ZShzdHlsZS50ZXh0Q29tYmluZSgpKTsKICAgICAg ICAgY2FzZSBDU1NQcm9wZXJ0eVdlYmtpdFRleHRPcmllbnRhdGlvbjoKZGlmZiAtLWdpdCBhL0xh eW91dFRlc3RzL2ltcG9ydGVkL3czYy9DaGFuZ2VMb2cgYi9MYXlvdXRUZXN0cy9pbXBvcnRlZC93 M2MvQ2hhbmdlTG9nCmluZGV4IDY4MzE2MWMxMTYwMGEwZjg5ZDQ3NWU1YzZmYTg4ZTQ0Mjg2MGRh ZGUuLjdlOTliOTc2MmZiMzEwZTQ1ZmFkMWY3YTg2YTkyYjNiNDdiN2Y3ZTUgMTAwNjQ0Ci0tLSBh L0xheW91dFRlc3RzL2ltcG9ydGVkL3czYy9DaGFuZ2VMb2cKKysrIGIvTGF5b3V0VGVzdHMvaW1w b3J0ZWQvdzNjL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0IEBACisyMDIyLTA1LTEwICBPcmlvbCBC cnVmYXUgIDxvYnJ1ZmF1QGlnYWxpYS5jb20+CisKKyAgICAgICAgW2Nzc29tXSBTZXJpYWxpemUg Y29tcHV0ZWQgJy13ZWJraXQtdGV4dC1jb21iaW5lOiBub25lJworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjM5OTg5CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVXBkYXRlIHNvbWUgdGVzdCBleHBlY3RhdGlv bnMuCisKKyAgICAgICAgKiB3ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzc29tL3NlcmlhbGl6ZS1h bGwtbG9uZ2hhbmRzLWV4cGVjdGVkLnR4dDoKKwogMjAyMi0wNS0wOSAgT3Jpb2wgQnJ1ZmF1ICA8 b2JydWZhdUBpZ2FsaWEuY29tPgogCiAgICAgICAgIFtjc3NvbV0gRG9uJ3QgaW5kZXggcGVyc3Bl Y3RpdmUvdHJhbnNmb3JtLW9yaWdpbi0qIGluIGNvbXB1dGVkIHN0eWxlcwpkaWZmIC0tZ2l0IGEv TGF5b3V0VGVzdHMvaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3Nzb20vc2Vy aWFsaXplLWFsbC1sb25naGFuZHMtZXhwZWN0ZWQudHh0IGIvTGF5b3V0VGVzdHMvaW1wb3J0ZWQv dzNjL3dlYi1wbGF0Zm9ybS10ZXN0cy9jc3MvY3Nzb20vc2VyaWFsaXplLWFsbC1sb25naGFuZHMt ZXhwZWN0ZWQudHh0CmluZGV4IDY2Y2E0NjFkNGZlZWU0YWJhMzZiMWRmZjIxMGRmMzBjMmM4MWVl NmIuLjMyZTU3MDlkZjE5ODE0MThmNDcxNjg0ZjA5OGViYzhhYTJkOWFhMWUgMTAwNjQ0Ci0tLSBh L0xheW91dFRlc3RzL2ltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvY3NzL2Nzc29tL3Nl cmlhbGl6ZS1hbGwtbG9uZ2hhbmRzLWV4cGVjdGVkLnR4dAorKysgYi9MYXlvdXRUZXN0cy9pbXBv cnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2Nzcy9jc3NvbS9zZXJpYWxpemUtYWxsLWxvbmdo YW5kcy1leHBlY3RlZC50eHQKQEAgLTEsNCArMSw0IEBACiAKIFBBU1MgU3BlY2lmaWVkIHN0eWxl Ci1GQUlMIENvbXB1dGVkIHN0eWxlIGFzc2VydF9hcnJheV9lcXVhbHM6IGxlbmd0aHMgZGlmZmVy LCBleHBlY3RlZCBhcnJheSBbXSBsZW5ndGggMCwgZ290IFsic2l6ZSIsICItd2Via2l0LXRleHQt Y29tYmluZSJdIGxlbmd0aCAyCitGQUlMIENvbXB1dGVkIHN0eWxlIGFzc2VydF9hcnJheV9lcXVh bHM6IGxlbmd0aHMgZGlmZmVyLCBleHBlY3RlZCBhcnJheSBbXSBsZW5ndGggMCwgZ290IFsic2l6 ZSJdIGxlbmd0aCAxCiAK