Dark Mode: In Notes, list item becomes invisible in dark mode after outdenting
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2020 02:56:32 +0000 (02:56 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2020 02:56:32 +0000 (02:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207676

Reviewed by Wenson Hsieh and Timothy Hatcher.

Source/WebCore:

The bug was caused by EditingStyle::inverseTransformColorIfNeeded converting -apple-system-label to
transparent color in ReplaceSelectionCommand when InsertListCommand invokes moveParagraphs.

This patch fixes the bug in EditingStyle::inverseTransformColorIfNeeded by treating any semantic color
name or semantic RGB color value as if the color was not specified.

It also fixes the bug that removeStyleFromRulesAndContext was incapable of removing superflous semantic
color names that appear in the inline since the context's computed style only contain RGB values by
replacing the inline style's color values with that of the computed style. This fix is necessary to
eliminate -apple-system-label in the pasted content, which can cause issues when such a content is
sync'ed to other devices via iCloud, etc...

Tests: PasteHTML.TransformColorsOfDarkContentButNotSemanticColor
       PasteHTML.DoesNotTransformColorsOfLightContentDuringOutdent

* editing/EditingStyle.cpp:
(WebCore::EditingStyle::removeStyleFromRulesAndContext):
(WebCore::EditingStyle::inverseTransformColorIfNeeded):

Tools:

Added regression tests for pasting content with -apple-system-label and outdenting content.

* TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:
(PasteHTML.TransformColorsOfDarkContentButNotSemanticColor):
(PasteHTML.DoesNotTransformColorsOfLightContentDuringOutdent):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256580 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm

index e177915..2e0633c 100644 (file)
@@ -1,3 +1,29 @@
+2020-02-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Dark Mode: In Notes, list item becomes invisible in dark mode after outdenting
+        https://bugs.webkit.org/show_bug.cgi?id=207676
+
+        Reviewed by Wenson Hsieh and Timothy Hatcher.
+
+        The bug was caused by EditingStyle::inverseTransformColorIfNeeded converting -apple-system-label to
+        transparent color in ReplaceSelectionCommand when InsertListCommand invokes moveParagraphs.
+
+        This patch fixes the bug in EditingStyle::inverseTransformColorIfNeeded by treating any semantic color
+        name or semantic RGB color value as if the color was not specified.
+
+        It also fixes the bug that removeStyleFromRulesAndContext was incapable of removing superflous semantic
+        color names that appear in the inline since the context's computed style only contain RGB values by
+        replacing the inline style's color values with that of the computed style. This fix is necessary to
+        eliminate -apple-system-label in the pasted content, which can cause issues when such a content is
+        sync'ed to other devices via iCloud, etc...
+
+        Tests: PasteHTML.TransformColorsOfDarkContentButNotSemanticColor
+               PasteHTML.DoesNotTransformColorsOfLightContentDuringOutdent
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::removeStyleFromRulesAndContext):
+        (WebCore::EditingStyle::inverseTransformColorIfNeeded):
+
 2020-02-13  Brent Fulgham  <bfulgham@apple.com>
 
         REGRESSION (r255961): Default state for data URL handling is incorrect
index 9990912..7ff1c32 100644 (file)
@@ -1366,6 +1366,30 @@ void EditingStyle::removeStyleFromRulesAndContext(StyledElement& element, Node*
         if (!computedStyle->m_mutableStyle->getPropertyCSSValue(CSSPropertyBackgroundColor))
             computedStyle->m_mutableStyle->setProperty(CSSPropertyBackgroundColor, CSSValueTransparent);
 
+        RefPtr<EditingStyle> computedStyleOfElement;
+        auto replaceSemanticColorWithComputedValue = [&](const CSSPropertyID id) {
+            auto color = m_mutableStyle->propertyAsColor(id);
+            if (!color || (color->isVisible() && !color->isSemantic()))
+                return;
+
+            if (!computedStyleOfElement)
+                computedStyleOfElement = EditingStyle::create(&element, EditingPropertiesInEffect);
+
+            if (!computedStyleOfElement->m_mutableStyle)
+                return;
+
+            auto computedValue = computedStyleOfElement->m_mutableStyle->getPropertyValue(id);
+            if (!computedValue)
+                return;
+
+            m_mutableStyle->setProperty(id, computedValue);
+        };
+
+        // Replace semantic color identifiers like -apple-system-label with RGB values so that comparsions in getPropertiesNotIn below would work.
+        replaceSemanticColorWithComputedValue(CSSPropertyColor);
+        replaceSemanticColorWithComputedValue(CSSPropertyCaretColor);
+        replaceSemanticColorWithComputedValue(CSSPropertyBackgroundColor);
+
         removePropertiesInStyle(computedStyle->m_mutableStyle.get(), styleFromMatchedRules.get());
         m_mutableStyle = getPropertiesNotIn(*m_mutableStyle, *computedStyle->m_mutableStyle);
     }
@@ -1587,10 +1611,17 @@ Ref<EditingStyle> EditingStyle::inverseTransformColorIfNeeded(Element& element)
     if (!m_mutableStyle || !renderer || !renderer->style().hasAppleColorFilter())
         return *this;
 
-    bool hasColor = m_mutableStyle->getPropertyCSSValue(CSSPropertyColor);
-    bool hasCaretColor = m_mutableStyle->getPropertyCSSValue(CSSPropertyCaretColor);
-    bool hasBackgroundColor = m_mutableStyle->getPropertyCSSValue(CSSPropertyBackgroundColor);
-    if (!hasColor && !hasCaretColor && !hasBackgroundColor)
+    auto colorForPropertyIfInvertible = [&](CSSPropertyID id) -> Optional<Color> {
+        auto color = m_mutableStyle->propertyAsColor(id);
+        if (!color || !color->isVisible() || color->isSemantic())
+            return WTF::nullopt;
+        return color;
+    };
+
+    auto color = colorForPropertyIfInvertible(CSSPropertyColor);
+    auto caretColor = colorForPropertyIfInvertible(CSSPropertyCaretColor);
+    auto backgroundColor = colorForPropertyIfInvertible(CSSPropertyBackgroundColor);
+    if (!color && !caretColor && !backgroundColor)
         return *this;
 
     auto styleWithInvertedColors = copy();
@@ -1603,13 +1634,13 @@ Ref<EditingStyle> EditingStyle::inverseTransformColorIfNeeded(Element& element)
         styleWithInvertedColors->m_mutableStyle->setProperty(propertyID, newColor.cssText());
     };
 
-    if (hasColor)
+    if (color)
         invertedColor(CSSPropertyColor);
 
-    if (hasCaretColor)
+    if (caretColor)
         invertedColor(CSSPropertyCaretColor);
 
-    if (hasBackgroundColor)
+    if (backgroundColor)
         invertedColor(CSSPropertyBackgroundColor);
 
     return styleWithInvertedColors;
index 1be74ff..87b38ee 100644 (file)
@@ -1,3 +1,16 @@
+2020-02-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Dark Mode: In Notes, list item becomes invisible in dark mode after outdenting
+        https://bugs.webkit.org/show_bug.cgi?id=207676
+
+        Reviewed by Wenson Hsieh and Timothy Hatcher.
+
+        Added regression tests for pasting content with -apple-system-label and outdenting content.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/PasteHTML.mm:
+        (PasteHTML.TransformColorsOfDarkContentButNotSemanticColor):
+        (PasteHTML.DoesNotTransformColorsOfLightContentDuringOutdent):
+
 2020-02-13  Alex Christensen  <achristensen@webkit.org>
 
         _WKResourceLoadInfo should conform to NSSecureCoding
index b943d00..58a471e 100644 (file)
@@ -420,6 +420,42 @@ TEST(PasteHTML, DoesNotTransformColorsOfLightContent)
     EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"rich.querySelector('span').style.color"], @"rgb(101, 101, 101)");
 }
 
+TEST(PasteHTML, TransformColorsOfDarkContentButNotSemanticColor)
+{
+    auto webView = createWebViewWithCustomPasteboardDataSetting(true, true);
+    [webView forceDarkMode];
+
+    [webView synchronouslyLoadTestPageNamed:@"rich-color-filtered"];
+
+    [webView stringByEvaluatingJavaScript:@"document.body.style.color = '-apple-system-label';"];
+    writeHTMLToPasteboard(@"<span style='color: -apple-system-label'>hello</span><b style='color: #eee'>world</b>");
+
+    [webView stringByEvaluatingJavaScript:@"selectRichText()"];
+    [webView paste:nil];
+
+#if USE(APPKIT)
+    EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"rich.innerHTML"], @"hello<b style=\"color: rgb(21, 21, 21);\">world</b>");
+#else
+    EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"rich.innerHTML"],
+        @"<span style=\"-webkit-text-size-adjust: auto;\">hello</span><b style=\"-webkit-text-size-adjust: auto; color: rgb(21, 21, 21);\">world</b>");
+#endif
+}
+
+TEST(PasteHTML, DoesNotTransformColorsOfLightContentDuringOutdent)
+{
+    auto webView = createWebViewWithCustomPasteboardDataSetting(true, true);
+    [webView forceDarkMode];
+
+    [webView synchronouslyLoadTestPageNamed:@"rich-color-filtered"];
+
+    [webView stringByEvaluatingJavaScript:@"document.body.style = `color: -apple-system-label; caret-color: -apple-system-secondary-label; background-color: -apple-system-text-background;`;"];
+    [webView stringByEvaluatingJavaScript:@"rich.innerHTML = `<ul><li>hello</li><ul><li id='target'>world</li></ul></ul>`;"];
+
+    [webView stringByEvaluatingJavaScript:@"getSelection().setPosition(target, 0); document.execCommand('outdent');"];
+
+    EXPECT_WK_STREQ([webView stringByEvaluatingJavaScript:@"rich.innerHTML"], @"<ul><li>hello</li><li>world</li></ul>");
+}
+
 #endif // ENABLE(DARK_MODE_CSS) && HAVE(OS_DARK_MODE_SUPPORT)
 
 #endif // PLATFORM(COCOA)