Title: [260173] trunk
Revision
260173
Author
mmaxfi...@apple.com
Date
2020-04-15 22:57:41 -0700 (Wed, 15 Apr 2020)

Log Message

[Cocoa] Password obscuring dots drawn with the system font are too small
https://bugs.webkit.org/show_bug.cgi?id=209692
<rdar://problem/60788385>

Reviewed by Darin Adler.

Source/WebCore:

The system font's U+2022 BULLET glyph got smaller. Instead, we should match
the native platform's behavior of using U+F79A. However, U+F79A is a PUA
character, meaning different fonts will draw it in arbitrary different ways.
Therefore, we should only use this character if we're drawing it with the
system font. Otherwise, we can take the old codepath and use U+2022 BULLET.

Tests: fast/text/text-security-disc-bullet-pua.html
       platform/mac/fast/text/text-security-disc-bullet-pua-mac.html
       platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html
       platform/ios/fast/text/text-security-disc-bullet-pua-ios-old.html

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::text const):
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::constructTextRun):
* rendering/SimpleLineLayout.cpp:
(WebCore::SimpleLineLayout::canUseForStyle):
* rendering/SimpleLineLayoutCoverage.cpp:
(WebCore::SimpleLineLayout::printReason):
* rendering/SimpleLineLayoutCoverage.h:
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::computeTextSecurityDiscShouldUsePUACodePoint const):
* rendering/style/RenderStyle.h:

LayoutTests:

* fast/text/text-security-disc-bullet-pua-expected.html: Added.
* fast/text/text-security-disc-bullet-pua.html: Added.
* platform/ios/fast/text/text-security-disc-bullet-pua-ios-new-expected.html: Added.
* platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html: Added.
* platform/ios/fast/text/text-security-disc-bullet-pua-ios-old-expected.html: Added.
* platform/ios/fast/text/text-security-disc-bullet-pua-ios-old.html: Added.
* platform/mac/fast/text/text-security-disc-bullet-pua-mac-expected.html: Added.
* platform/mac/fast/text/text-security-disc-bullet-pua-mac.html: Added.
* platform/ios/TestExpectations:
* platform/mac/TestExpectations:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (260172 => 260173)


--- trunk/LayoutTests/ChangeLog	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/LayoutTests/ChangeLog	2020-04-16 05:57:41 UTC (rev 260173)
@@ -1,3 +1,22 @@
+2020-04-15  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [Cocoa] Password obscuring dots drawn with the system font are too small
+        https://bugs.webkit.org/show_bug.cgi?id=209692
+        <rdar://problem/60788385>
+
+        Reviewed by Darin Adler.
+
+        * fast/text/text-security-disc-bullet-pua-expected.html: Added.
+        * fast/text/text-security-disc-bullet-pua.html: Added.
+        * platform/ios/fast/text/text-security-disc-bullet-pua-ios-new-expected.html: Added.
+        * platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html: Added.
+        * platform/ios/fast/text/text-security-disc-bullet-pua-ios-old-expected.html: Added.
+        * platform/ios/fast/text/text-security-disc-bullet-pua-ios-old.html: Added.
+        * platform/mac/fast/text/text-security-disc-bullet-pua-mac-expected.html: Added.
+        * platform/mac/fast/text/text-security-disc-bullet-pua-mac.html: Added.
+        * platform/ios/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2020-04-15  Simon Fraser  <simon.fra...@apple.com>
 
         [Async overflow scroll] background-attachment:fixed needs to disable async overflow scrolling

Added: trunk/LayoutTests/fast/text/text-security-disc-bullet-pua-expected.html (0 => 260173)


--- trunk/LayoutTests/fast/text/text-security-disc-bullet-pua-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/text-security-disc-bullet-pua-expected.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui;">&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;</div>
+<div style="font: 48px serif;">&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;</div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/text/text-security-disc-bullet-pua.html (0 => 260173)


--- trunk/LayoutTests/fast/text/text-security-disc-bullet-pua.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/text-security-disc-bullet-pua.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui; -webkit-text-security: disc;">abcdefg</div>
+<div style="font: 48px serif; -webkit-text-security: disc;">abcdefg</div>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/ios/TestExpectations (260172 => 260173)


--- trunk/LayoutTests/platform/ios/TestExpectations	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/LayoutTests/platform/ios/TestExpectations	2020-04-16 05:57:41 UTC (rev 260173)
@@ -3524,3 +3524,7 @@
 
 webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-007.html [ Skip ]
 webkit.org/b/209734 imported/w3c/web-platform-tests/css/selectors/focus-visible-009.html [ Skip ]
+
+# Certain versions of iOS use different text security characters.
+webkit.org/b/209692 platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html [ ImageOnlyFailure ]
+webkit.org/b/209692 fast/text/text-security-disc-bullet-pua.html [ ImageOnlyFailure ]

Added: trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-new-expected.html (0 => 260173)


--- trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-new-expected.html	                        (rev 0)
+++ trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-new-expected.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui;">&#xF79A;&#xF79A;&#xF79A;&#xF79A;&#xF79A;&#xF79A;&#xF79A;</div>
+<div style="font: 48px serif;">&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;</div>
+</body>
+</html>

Added: trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html (0 => 260173)


--- trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html	                        (rev 0)
+++ trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui; -webkit-text-security: disc;">abcdefg</div>
+<div style="font: 48px serif; -webkit-text-security: disc;">abcdefg</div>
+</body>
+</html>

Added: trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-old-expected.html (0 => 260173)


--- trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-old-expected.html	                        (rev 0)
+++ trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-old-expected.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui;">&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;</div>
+<div style="font: 48px serif;">&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;&#x25CF;</div>
+</body>
+</html>

Added: trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-old.html (0 => 260173)


--- trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-old.html	                        (rev 0)
+++ trunk/LayoutTests/platform/ios/fast/text/text-security-disc-bullet-pua-ios-old.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui; -webkit-text-security: disc;">abcdefg</div>
+<div style="font: 48px serif; -webkit-text-security: disc;">abcdefg</div>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/mac/TestExpectations (260172 => 260173)


--- trunk/LayoutTests/platform/mac/TestExpectations	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2020-04-16 05:57:41 UTC (rev 260173)
@@ -1975,4 +1975,7 @@
 
 webkit.org/b/207160 css2.1/20110323/replaced-intrinsic-ratio-001.htm [ Pass Failure ]
 
-webkit.org/b/210351 scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe.html [ Pass Timeout ]
\ No newline at end of file
+webkit.org/b/210351 scrollingcoordinator/mac/latching/horizontal-overflow-back-swipe.html [ Pass Timeout ]
+
+# Certain versions of macOS use different text security characters.
+webkit.org/b/209692 [ Sierra HighSierra Mojave Catalina ] platform/mac/fast/text/text-security-disc-bullet-pua-mac.html [ ImageOnlyFailure ]

Added: trunk/LayoutTests/platform/mac/fast/text/text-security-disc-bullet-pua-mac-expected.html (0 => 260173)


--- trunk/LayoutTests/platform/mac/fast/text/text-security-disc-bullet-pua-mac-expected.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/text/text-security-disc-bullet-pua-mac-expected.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui;">&#xF79A;&#xF79A;&#xF79A;&#xF79A;&#xF79A;&#xF79A;&#xF79A;</div>
+<div style="font: 48px serif;">&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;&#x2022;</div>
+</body>
+</html>

Added: trunk/LayoutTests/platform/mac/fast/text/text-security-disc-bullet-pua-mac.html (0 => 260173)


--- trunk/LayoutTests/platform/mac/fast/text/text-security-disc-bullet-pua-mac.html	                        (rev 0)
+++ trunk/LayoutTests/platform/mac/fast/text/text-security-disc-bullet-pua-mac.html	2020-04-16 05:57:41 UTC (rev 260173)
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>This test makes sure that password fields get drawn with the correct text-security character.</p>
+<div style="font: 48px system-ui; -webkit-text-security: disc;">abcdefg</div>
+<div style="font: 48px serif; -webkit-text-security: disc;">abcdefg</div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (260172 => 260173)


--- trunk/Source/WebCore/ChangeLog	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/ChangeLog	2020-04-16 05:57:41 UTC (rev 260173)
@@ -1,3 +1,35 @@
+2020-04-15  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [Cocoa] Password obscuring dots drawn with the system font are too small
+        https://bugs.webkit.org/show_bug.cgi?id=209692
+        <rdar://problem/60788385>
+
+        Reviewed by Darin Adler.
+
+        The system font's U+2022 BULLET glyph got smaller. Instead, we should match
+        the native platform's behavior of using U+F79A. However, U+F79A is a PUA
+        character, meaning different fonts will draw it in arbitrary different ways.
+        Therefore, we should only use this character if we're drawing it with the
+        system font. Otherwise, we can take the old codepath and use U+2022 BULLET.
+
+        Tests: fast/text/text-security-disc-bullet-pua.html
+               platform/mac/fast/text/text-security-disc-bullet-pua-mac.html
+               platform/ios/fast/text/text-security-disc-bullet-pua-ios-new.html
+               platform/ios/fast/text/text-security-disc-bullet-pua-ios-old.html
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::text const):
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::constructTextRun):
+        * rendering/SimpleLineLayout.cpp:
+        (WebCore::SimpleLineLayout::canUseForStyle):
+        * rendering/SimpleLineLayoutCoverage.cpp:
+        (WebCore::SimpleLineLayout::printReason):
+        * rendering/SimpleLineLayoutCoverage.h:
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::computeTextSecurityDiscShouldUsePUACodePoint const):
+        * rendering/style/RenderStyle.h:
+
 2020-04-15  Jer Noble  <jer.no...@apple.com>
 
         REGRESSION (r260102): ASSERTION FAILED: m_arbitrators.contains(proxy) in WebKit::SharedArbitrator::endRoutingArbitrationForArbitrator

Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (260172 => 260173)


--- trunk/Source/WebCore/rendering/InlineTextBox.cpp	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp	2020-04-16 05:57:41 UTC (rev 260173)
@@ -1414,17 +1414,23 @@
 
 String InlineTextBox::text(bool ignoreCombinedText, bool ignoreHyphen) const
 {
+    String result;
     if (auto* combinedText = this->combinedText()) {
         if (ignoreCombinedText)
-            return renderer().text().substring(m_start, m_len);
-        return combinedText->combinedStringForRendering();
-    }
-    if (hasHyphen()) {
+            result = renderer().text().substring(m_start, m_len);
+        else
+            result = combinedText->combinedStringForRendering();
+    } else if (hasHyphen()) {
         if (ignoreHyphen)
-            return renderer().text().substring(m_start, m_len);
-        return makeString(StringView(renderer().text()).substring(m_start, m_len), lineStyle().hyphenString());
-    }
-    return renderer().text().substring(m_start, m_len);
+            result = renderer().text().substring(m_start, m_len);
+        else
+            result = makeString(StringView(renderer().text()).substring(m_start, m_len), lineStyle().hyphenString());
+    } else
+        result = renderer().text().substring(m_start, m_len);
+
+    // This works because this replacement doesn't affect string indices. We're replacing a single Unicode code unit with another Unicode code unit.
+    // How convenient.
+    return RenderBlock::updateSecurityDiscCharacters(lineStyle(), WTFMove(result));
 }
 
 inline const RenderCombineText* InlineTextBox::combinedText() const

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (260172 => 260173)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2020-04-16 05:57:41 UTC (rev 260173)
@@ -3124,7 +3124,13 @@
         if (flags & RespectDirectionOverride)
             directionalOverride |= isOverride(style.unicodeBidi());
     }
-    return TextRun(stringView, 0, 0, expansion, textDirection, directionalOverride);
+
+    // This works because:
+    // 1. TextRun owns its text string. Its member is a String, not a StringView
+    // 2. This replacement doesn't affect string indices. We're replacing a single Unicode code unit with another Unicode code unit.
+    // How convenient.
+    auto updatedString = RenderBlock::updateSecurityDiscCharacters(style, stringView.toStringWithoutCopying());
+    return TextRun(WTFMove(updatedString), 0, 0, expansion, textDirection, directionalOverride);
 }
 
 TextRun RenderBlock::constructTextRun(const String& string, const RenderStyle& style, ExpansionBehavior expansion, TextRunFlags flags)
@@ -3502,5 +3508,29 @@
     LayoutPoint childPoint = flipForWritingModeForChild(legend, accumulatedOffset);
     return legend->nodeAtPoint(request, result, locationInContainer, childPoint, childHitTest);
 }
+
+String RenderBlock::updateSecurityDiscCharacters(const RenderStyle& style, String&& string)
+{
+#if !PLATFORM(COCOA)
+    return WTFMove(string);
+#else
+    if (style.textSecurity() == TextSecurity::None)
+        return WTFMove(string);
+    // This PUA character in the system font is used to render password field dots on Cocoa platforms.
+    constexpr UChar textSecurityDiscPUACodePoint = 0xF79A;
+    auto& font = style.fontCascade().primaryFont();
+    if (!(font.platformData().isSystemFont() && font.glyphForCharacter(textSecurityDiscPUACodePoint)))
+        return WTFMove(string);
+
+    // See RenderText::setRenderedText()
+#if PLATFORM(IOS_FAMILY)
+    constexpr UChar discCharacterToReplace = blackCircle;
+#else
+    constexpr UChar discCharacterToReplace = bullet;
+#endif
+
+    return string.replace(discCharacterToReplace, textSecurityDiscPUACodePoint);
+#endif
+}
     
 } // namespace WebCore

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (260172 => 260173)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2020-04-16 05:57:41 UTC (rev 260173)
@@ -318,6 +318,8 @@
 
     virtual bool shouldResetChildLogicalHeightBeforeLayout(const RenderBox&) const { return false; }
 
+    static String updateSecurityDiscCharacters(const RenderStyle&, String&&);
+
 protected:
     RenderFragmentedFlow* locateEnclosingFragmentedFlow() const override;
     void willBeDestroyed() override;

Modified: trunk/Source/WebCore/rendering/RenderText.cpp (260172 => 260173)


--- trunk/Source/WebCore/rendering/RenderText.cpp	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/rendering/RenderText.cpp	2020-04-16 05:57:41 UTC (rev 260173)
@@ -1166,6 +1166,8 @@
     if (style.textTransform() != TextTransform::None)
         m_text = applyTextTransform(style, m_text, previousCharacter());
 
+    // At rendering time, if certain fonts are used, these characters get swapped out with higher-quality PUA characters.
+    // See RenderBlock::updateSecurityDiscCharacters().
     switch (style.textSecurity()) {
     case TextSecurity::None:
         break;

Modified: trunk/Source/WebCore/rendering/SimpleLineLayout.cpp (260172 => 260173)


--- trunk/Source/WebCore/rendering/SimpleLineLayout.cpp	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/rendering/SimpleLineLayout.cpp	2020-04-16 05:57:41 UTC (rev 260173)
@@ -240,6 +240,10 @@
         SET_REASON_AND_RETURN_IF_NEEDED(FlowHasNonAutoLineBreak, reasons, includeReasons);
     if (style.nbspMode() != NBSPMode::Normal)
         SET_REASON_AND_RETURN_IF_NEEDED(FlowHasWebKitNBSPMode, reasons, includeReasons);
+    // Special handling of text-security:disc is not yet implemented in the simple line layout code path.
+    // See RenderBlock::updateSecurityDiscCharacters.
+    if (style.textSecurity() != TextSecurity::None)
+        SET_REASON_AND_RETURN_IF_NEEDED(FlowHasTextSecurity, reasons, includeReasons);
     if (style.hyphens() == Hyphens::Auto) {
         auto textReasons = canUseForText(style.hyphenString(), style.fontCascade(), WTF::nullopt, false, includeReasons);
         if (textReasons != NoReason)

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutCoverage.cpp (260172 => 260173)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutCoverage.cpp	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutCoverage.cpp	2020-04-16 05:57:41 UTC (rev 260173)
@@ -135,6 +135,9 @@
     case FlowHasNonAutoLineBreak:
         stream << "line-break is not auto";
         break;
+    case FlowHasTextSecurity:
+        stream << "text-security is not none";
+        break;
     case FlowHasSVGFont:
         stream << "SVG font";
         break;

Modified: trunk/Source/WebCore/rendering/SimpleLineLayoutCoverage.h (260172 => 260173)


--- trunk/Source/WebCore/rendering/SimpleLineLayoutCoverage.h	2020-04-16 05:53:41 UTC (rev 260172)
+++ trunk/Source/WebCore/rendering/SimpleLineLayoutCoverage.h	2020-04-16 05:57:41 UTC (rev 260173)
@@ -66,6 +66,7 @@
     FlowHasTextFillBox                    = 1LLU  << 28,
     FlowHasBorderFitLines                 = 1LLU  << 29,
     FlowHasNonAutoLineBreak               = 1LLU  << 30,
+    FlowHasTextSecurity                   = 1LLU  << 31,
     FlowHasSVGFont                        = 1LLU  << 32,
     FlowTextIsEmpty                       = 1LLU  << 33,
     FlowTextHasSoftHyphen                 = 1LLU  << 34,
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to