Bug 279008 - text-emphasis only uses the first character in the grapheme cluster
Summary: text-emphasis only uses the first character in the grapheme cluster
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: Safari Technology Preview
Hardware: Mac (Apple Silicon) Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2024-09-02 00:00 PDT by yisibl
Modified: 2024-09-13 08:10 PDT (History)
8 users (show)

See Also:

Attachments
text-emphasis-emoji (741.18 KB, image/png)
2024-09-02 00:00 PDT, yisibl 		no flags Details
test case (198 bytes, text/html)
2024-09-02 12:38 PDT, Alexey Proskuryakov 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description yisibl 2024-09-02 00:00:03 PDT 
Created attachment 472401 [details]
text-emphasis-emoji

Open the following code in Safari and you'll see that the text-emphasis is only rendered to `U+2764`

data:text/html;charset=UTF-8,<!doctype html>
<style>
.foo {
	font-size: 52px;
	border: solid 1px blue;
	text-emphasis-style: "❤️";
	display: inline;
}
</style>
<div class="foo">test emoji❤️</div>

❤️ This emoji actually contains two characters:

`U+2764`
`U+FE0F`
Comment 1 Radar WebKit Bug Importer 2024-09-02 02:08:03 PDT 
<rdar://problem/135133566>
Comment 2 Alexey Proskuryakov 2024-09-02 12:37:55 PDT 
This is not just emoji with variation selectors. Same issue with e.g. decomposed é, or  regional indicators like 🇷🇺.
Comment 3 Alexey Proskuryakov 2024-09-02 12:38:29 PDT 
Created attachment 472406 [details]
test case

Same test as attachment.
Comment 4 zalan 2024-09-02 13:39:10 PDT 
Looks like drawEmphasisMarks is broken. using drawBidiText instead makes the bug go away (note that this is not a fix proposal)

diff --git a/Source/WebCore/rendering/TextPainter.cpp b/Source/WebCore/rendering/TextPainter.cpp
index 8a84ad92b70b..4be4f964b5b9 100644
--- a/Source/WebCore/rendering/TextPainter.cpp
+++ b/Source/WebCore/rendering/TextPainter.cpp
@@ -117,7 +117,7 @@ void TextPainter::paintTextOrEmphasisMarks(const FontCascade& font, const TextRu
     }
 
     if (!emphasisMark.isEmpty())
-        m_context.drawEmphasisMarks(font, textRun, emphasisMark, textOrigin + FloatSize(0, emphasisMarkOffset), startOffset, endOffset);
+        m_context.drawBidiText(font, TextRun { emphasisMark.string() }, textOrigin + FloatSize(0, emphasisMarkOffset));
     else if (startOffset || endOffset < textRun.length() || !m_glyphDisplayList)
         m_context.drawText(font, textRun, textOrigin, startOffset, endOffset);
     else {
Comment 5 zalan 2024-09-04 09:21:04 PDT 
Maybe we use the wrong font? Seeing this FIXME comment in getEmphasisMarkGlyphData does not give me too much confidence. 

// FIXME: This function may not work if the emphasis mark uses a complex script, but none of the
// standard emphasis marks do so.
std::optional<GlyphData> FontCascade::getEmphasisMarkGlyphData(const AtomString& mark) const