Bug 171664 - text-transform:uppercase in SVG causes misplaced letters
Summary: text-transform:uppercase in SVG causes misplaced letters
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat, InRadar
: 245248 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-05-04 09:13 PDT by Simon Fraser (smfr)
Modified: 2022-10-23 12:46 PDT (History)
13 users (show)

See Also:


Attachments
Testcase (834 bytes, text/html)
2017-06-17 10:28 PDT, Simon Fraser (smfr)
no flags Details
Safari 156 and STP 151 differs from other browsers (1016.77 KB, image/png)
2022-08-13 04:58 PDT, Ahmad Saleem
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2017-05-04 09:13:00 PDT
The 'M' is missing from the second line in this test case:

https://codepen.io/AmeliaBR/pen/61a8a4f3848023859253cb5341e17ef2?editors=1100
Comment 1 Radar WebKit Bug Importer 2017-05-04 09:14:16 PDT
<rdar://problem/31989978>
Comment 2 Simon Fraser (smfr) 2017-06-17 10:28:50 PDT
Created attachment 313200 [details]
Testcase
Comment 3 Simon Fraser (smfr) 2017-06-17 10:40:27 PDT
Any test-transform triggers this, even if using uppercase and the letters are already uppercase. Happens only when the text-transform is on the encoding <text>, not an individual <tspan>.
Comment 4 Simon Fraser (smfr) 2017-06-17 10:46:16 PDT
I think this is about whitespace collapsing. If you remove the whitespace between the  <tspan>s, the bug goes away.
Comment 5 Simon Fraser (smfr) 2017-06-17 10:56:02 PDT
In RenderText::styleDidChange() we call RenderText::setText(originalText(), true), which sets the text to RenderSVGInlineText::originalText() which adds a newline that is not included in RenderText::originalText(). I'm not sure why RenderSVGInlineText has to override originalText().
Comment 6 Simon Fraser (smfr) 2017-06-19 20:39:29 PDT
Antti added RenderSVGInlineText::originalText() in https://trac.webkit.org/changeset/160259/webkit, but without an explanation for why SVG is special.
Comment 7 Simon Fraser (smfr) 2017-06-19 20:59:05 PDT
Possible patch:

diff --git a/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp b/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp
index 5aebea8abc3a9ede361f2edfc3e20d5ea1e9b611..a5af6fe694a4982ec697fed6339d32be624f474e 100644
--- a/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp
+++ b/Source/WebCore/rendering/svg/RenderSVGInlineText.cpp
@@ -78,7 +78,7 @@ String RenderSVGInlineText::originalText() const
 
 void RenderSVGInlineText::setRenderedText(const String& text)
 {
-    RenderText::setRenderedText(text);
+    RenderText::setRenderedText(applySVGWhitespaceRules(text, style().whiteSpace() == PRE));
     if (auto* textAncestor = RenderSVGText::locateRenderSVGTextAncestor(*this))
         textAncestor->subtreeTextDidChange(this);
 }
Comment 8 Ahmad Saleem 2022-08-13 04:58:53 PDT
Created attachment 461574 [details]
Safari 156 and STP 151 differs from other browsers

I am able to reproduce this issue in Safari 15.6 and Safari TP 151 and as can bee seen in the attached screenshot "M" is on top line and not next line like other browsers.

Just wanted to share updated testing results. Thanks!
Comment 9 Ahmad Saleem 2022-09-15 16:07:21 PDT
*** Bug 245248 has been marked as a duplicate of this bug. ***
Comment 10 Ahmad Saleem 2022-10-23 12:46:08 PDT
It works and compile if you do:

void RenderSVGInlineText::setRenderedText(const String& text)
{
-    RenderText::setRenderedText(text);
+    RenderText::setRenderedText(applySVGWhitespaceRules(text, style().whiteSpace() == WhiteSpace::Pre));

But it has performance implications, my PR was here:

https://github.com/WebKit/WebKit/pull/4405

It has performance implications as mentioned by @Simon since above change will take assumption that all text has whitespace and run with it.

____

I had to close my PR due to mid changes clash, it got unintended changes from some other changes. Will try again in future but if someone has to take it up, they can.