1738560 - svg text elements not rendered when text-transform: uppercase is applied

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[David Chester]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:95.0) Gecko/20100101 Firefox/95.0

Steps to reproduce:

Load a page with the following HTML:

<svg>
  <text y="20" style="text-transform: uppercase">
    Hello, World!
  </text>
</svg>

Actual results:

Page is blank, with no text rendered.

Expected results:

Page should contain rendered text.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Web Painting' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[David Chester]

In the wild, this bug makes NYTimes' Spelling Bee game effectively unplayable: https://www.nytimes.com/puzzles/spelling-bee (behind a paywall)

Comment 3

[Alice0775 White]

I can reproduce the issue in Nightly95.0a1 Windows10.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=7fd59a7f04059ee17ee40dfcfa2ce74b263dffd4&tochange=4a265cea0290bdf392f1b9624b7df68622a66972

Comment 4

[Jonathan Kew [:jfkthame]]

Huh, that was..... unexpected!

Turns out we depend on BuildTextRunsScanner::BreakSink::Finish to finalize the transformed textrun when a text-transform is in effect, and the optimization to skip line-breaking work in SVG text results in skipping that and the textrun remains blank.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1738560 - When text-transform is in effect, ensure we finish building the transformed textrun even if we would otherwise skip the line-break scan (in SVG text). r=#layout-reviewers

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6033db719c8
When text-transform is in effect, ensure we finish building the transformed textrun even if we would otherwise skip the line-break scan (in SVG text). r=emilio

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/31473 for changes under testing/web-platform/tests

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/d6033db719c8

Comment 9

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 11

[Alice0775 White]

[Tracking Requested - why for this release]: SVG text not rendered if text-transform: uppercase is applied

Comment 12

[Jonathan Kew [:jfkthame]]

Comment on attachment 9248666 [details]
Bug 1738560 - When text-transform is in effect, ensure we finish building the transformed textrun even if we would otherwise skip the line-break scan (in SVG text). r=#layout-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: In SVG images, any text elements styled with text-transform are not displayed.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Just undoes part of the optimization from bug 1712780, in the case where text-transform is present.
• String changes made/needed:

Comment 13

[Pascal Chevrel:pascalc]

Comment on attachment 9248666 [details]
Bug 1738560 - When text-transform is in effect, ensure we finish building the transformed textrun even if we would otherwise skip the line-break scan (in SVG text). r=#layout-reviewers

Approved for 95 beta 3, thanks.

Comment 14

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/40d953d8a951

Comment 15

[Robert Longson [:longsonr]]

aren't there code paths where transformingFactory is nullptr? The new code calls transformingFactory.release() in all cases now doesn't it?

Comment 16

[Jonathan Kew [:jfkthame]]

(In reply to Robert Longson [:longsonr] from comment #15)

aren't there code paths where transformingFactory is nullptr? The new code calls transformingFactory.release() in all cases now doesn't it?

It does, but there's no harm in calling release() on an empty UniquePtr. All it does is tell the UniquePtr to abandon ownership of the pointer it's managing; it doesn't care if that pointer is actually null, as it isn't doing anything with it.