Reland "blink/css: Implement text-transform: full-width" [chromium/src : main]
Felipe Erias (Gerrit)
Felipe Erias added 1 comment![]( "Open in Gerrit")
Hello,
Unfortunately, the previous patch was reverted because these tests failed on Windows:
- css1/text_properties/text_transform.html
- css2.1/t1605-c545-txttrans-00-b-ag.html
- fast/css/textCapitalizeEdgeCases.html
- virtual/text-antialias/capitalize-empty-generated-string.html
- external/wpt/css/CSS2/text/text-transform-cap-002.xht
- external/wpt/css/CSS2/text/text-transform-cap-003.xht
- external/wpt/css/css-text/text-transform/text-transform-capitalize-033.html
- external/wpt/css/css-text/text-transform/text-transform-capitalize-036.html
https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Win10/160052/overview
I have rebased the CL and ran the tests again, and this time they passed.
Since those failing tests were not directly related to this CL, maybe it is fine to reland it?
I'll defer to your experience on this decision.
Thank you and best regards,
Felipe
Related details
- Kent Tamura
- Koji Ishii
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kent Tamura (Gerrit)
Kent Tamura added 1 comment![]( "Open in Gerrit")
Hello,
Unfortunately, the previous patch was reverted because these tests failed on Windows:
- css1/text_properties/text_transform.html
- css2.1/t1605-c545-txttrans-00-b-ag.html
- fast/css/textCapitalizeEdgeCases.html
- virtual/text-antialias/capitalize-empty-generated-string.html
- external/wpt/css/CSS2/text/text-transform-cap-002.xht
- external/wpt/css/CSS2/text/text-transform-cap-003.xht
- external/wpt/css/css-text/text-transform/text-transform-capitalize-033.html
- external/wpt/css/css-text/text-transform/text-transform-capitalize-036.html
https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Win10/160052/overview
I have rebased the CL and ran the tests again, and this time they passed.
Since those failing tests were not directly related to this CL, maybe it is fine to reland it?
I'll defer to your experience on this decision.
Thank you and best regards,
Felipe
These tests started failing when the original CL was merged, and stopped failing when it was reverted. Merging this without any changes will very likely make the tests start failing again.
Unfortunately we don't have a try bot for "WebKit Win10". Can you try to reproduce the failures with Windows 32bit build?
Related details
- Felipe Erias
- Koji Ishii
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Erias (Gerrit)
Felipe Erias voted and added 1 comment![]( "Open in Gerrit")
Votes added by Felipe Erias
| Commit-Queue | +1 |
Kent TamuraHello,
Unfortunately, the previous patch was reverted because these tests failed on Windows:
- css1/text_properties/text_transform.html
- css2.1/t1605-c545-txttrans-00-b-ag.html
- fast/css/textCapitalizeEdgeCases.html
- virtual/text-antialias/capitalize-empty-generated-string.html
- external/wpt/css/CSS2/text/text-transform-cap-002.xht
- external/wpt/css/CSS2/text/text-transform-cap-003.xht
- external/wpt/css/css-text/text-transform/text-transform-capitalize-033.html
- external/wpt/css/css-text/text-transform/text-transform-capitalize-036.html
https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Win10/160052/overview
I have rebased the CL and ran the tests again, and this time they passed.
Since those failing tests were not directly related to this CL, maybe it is fine to reland it?
I'll defer to your experience on this decision.
Thank you and best regards,
Felipe
These tests started failing when the original CL was merged, and stopped failing when it was reverted. Merging this without any changes will very likely make the tests start failing again.
Unfortunately we don't have a try bot for "WebKit Win10". Can you try to reproduce the failures with Windows 32bit build?
The builder machine that failed was `WebKit Win10`, the page says that is mirrored by the try builder `win_chromium_compile_rel_ng` but I don't know if that is enough for our purposes:
https://ci.chromium.org/ui/p/chromium/builders/ci/WebKit%20Win10
I have spent some time looking into this issue. This is my best guess at the moment.
Before the patch, the generated `ETextTransform` enum looked like this:
```
enum class ETextTransform : uint8_t {
kNone, // 0
kCapitalize, // 1
kUppercase, // 2
kLowercase, // 3
kMathAuto, // 4
};
```
When `full-width` was added, its value is inserted before most of the others in the generated enum:
```
enum class ETextTransform : uint8_t {
kNone, // 0
kFullWidth, // 1 (inserted here)
kCapitalize, // 2 (was 1)
kUppercase, // 3 (was 2)
kLowercase, // 4 (was 3)
kMathAuto, // 5 (was 4)
};
```
All of the failing tests check the interaction between `text-transform:capitalize` inside and `<span>` elements.
In particular, word boundary detection for `text-transform:capitalize` was broken when crossing <span> boundaries. The system was incorrectly treating characters after span boundaries as if they were at the start of a new word.
For example, in `external/wpt/css/CSS2/text/text-transform-cap-002.xht`:
```
T<span style="text-transform:capitalize">his text should be capitalized.</span>
```
- Expected: `This text should be capitalized.`
- Actual: `THis text should be capitalized.`
The `h` after the span boundary is incorrectly capitalized, suggesting the system thinks it is at a word boundary when it's actually in the middle of the word `"This"`.
My suspicion is that this builder might have been using stale intermediate files, so some code was using the new enum ordering while other code was using the old one.
I can not be 100% sure, but it seems that this might have reset the `previous_character` so the transform code assumed incorrectly that there was a word boundary.
Like I said, this is my best guess at the moment. If this is correct, then a full rebuild should be enough to fix the issue.
I have ran the standard CQ run plus additional Windows builds, including `win_chromium_compile_rel_ng`. No failures (except an infra failure in `win-arm64-rel`).
Best,
Felipe
Related details
- Kent Tamura
- Koji Ishii
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kent Tamura (Gerrit)
Kent Tamura added 1 comment![]( "Open in Gerrit")
the page says that is mirrored by the try builder `win_chromium_compile_rel_ng` but I don't know if that is enough for our purposes:
`win_chromium_compile_rel_ng` doesn't run any tests, so it's not helpful.
Have you tried to reproduce the issue on Windows 32bit locally? If the issue was not reproducible locally, it would be a build issue like the stale files and we should resolve or avoid the build issue. Otherwise, it might affect the production and we need to identify the root cause.
Related details
- Felipe Erias
- Koji Ishii
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |