Picking a color from the color panel for typing attributes needs to inverse transform...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Jul 2018 21:10:40 +0000 (21:10 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Jul 2018 21:10:40 +0000 (21:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187846

Patch by Ryosuke Niwa <rniwa@apple.com> on 2018-07-20
Reviewed by Simon Fraser.

Source/WebCore:

This patch refines the color inversion for editing introduced in r234005 so that font panels and WebKit embedders
can get and set inverted colors using attributed strings for a selected text in an editable region.

More specifically, when font panels or WebKit embedders set a font color or a background color via WebView's
_applyStyleToSelection and _applyEditingStyleToSelection, WebKit would automatically invert the color before inserting
into DOM so that the color visible to the user matches that's given to WebKit. Conversely,
[WebView attributedSubstringFromRange:nsRange] now return the color visible to the user, i.e. the color after
the color filter had been applied, so that some WebKit embedders can present the visually identical color to the user.

Because DOM never sees the color filter's effect in the computed style, etc... this patch reverts the change made to
StyleChange in r234005 to avoid inverting the color passed to execCommand. This makes editing apps which is unaware
of the dark mode or -apple-color-filter continue to function (because the color picker implemented in DOM will be
applied of the same color filter before being presented to the user).

Finally, this patch introduces a testing hook in applyCommandToFrame so that executing foreColor or backColor with
the soruce of CommandFromMenuOrKeyBinding would trigger the same code path as the one taken by Objective-C

Tests: editing/execCommand/set-backColor-with-color-filter-from-scripts.html
       editing/execCommand/set-foreColor-with-color-filter-from-scripts.html
       editing/mac/attributed-string/attribute-string-for-copy-with-color-filter.html
       editing/style/set-backColor-with-color-filter.html
       editing/style/set-foreColor-with-color-filter.html

* editing/EditingStyle.cpp:
(WebCore::EditingStyle::inverseTransformColorIfNeeded): Added.
(WebCore::StyleChange::StyleChange): Revert the change made in r234005 since this code is also used by execCommand
which is not desirable, and won't work for background color.
(WebCore::StyleChange::extractTextStyles): Ditto.
* editing/EditingStyle.h:
* editing/Editor.cpp:
(WebCore::Editor::applyStyle):
(WebCore::Editor::applyStyleToSelection): Call EditingStyle::inverseTransformColorIfNeeded when ColorFilterMode is
set to InvertColor.
* editing/Editor.h:
* editing/EditorCommand.cpp:
(WebCore::applyCommandToFrame): Added the aforementioned testing hook.
* editing/cocoa/HTMLConverter.mm:
(WebCore::editingAttributedStringFromRange): Take the color filtr into account. Some WebKit embedders use this
function to compute the font color in the selected text. Note that this function is mostly used for input methods
so the color doesn't really matter, and its implementation is distinct from that of HTMLConverter.

Source/WebKitLegacy/mac:

Invert the filtered font and background colors when using font panels, font pasteboard, and other Objective-C APIs.

* WebView/WebHTMLView.mm:
(-[WebHTMLView _applyStyleToSelection:withUndoAction:]): Share code with _applyEditingStyleToSelection.
(-[WebHTMLView _applyEditingStyleToSelection:withUndoAction:]):

Tools:

Fixed the bug that testRunner.execCommand was using the second argument as the value.

The second argument, aShowDefaultUI, should always be ignored in testRunner.execCommand,
and the third argument should be used as the value. DumpRenderTree's implementation does this already.

* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::execCommand):
* WebKitTestRunner/InjectedBundle/TestRunner.h:

LayoutTests:

Added tests to make sure foreground or background colors specified in execCommand don't get inverted.

Fixed the test which was asserting that textInputController.attributedSubstringFromRange doesn't invert the color
since that's exactly what WebKit embedders uses to compute the style for color pickers.

Added new tests for copy & paste case using textInputController.legacyAttributedString.

* editing/execCommand/set-backColor-with-color-filter-from-scripts-expected.txt: Added.
* editing/execCommand/set-backColor-with-color-filter-from-scripts.html: Added.
* editing/execCommand/set-foreColor-with-color-filter-from-scripts-expected.txt: Added.
* editing/execCommand/set-foreColor-with-color-filter-from-scripts.html: Added.
* editing/mac/attributed-string/attrib-string-range-with-color-filter-expected.txt:
* editing/mac/attributed-string/attrib-string-range-with-color-filter.html: This test uses attributedSubstringFromRange,
which implemented using editingAttributedStringFromRange in WebCore. Since this is exactly what WebKit embedders uses
to compute the current style of the selected text, we need to invert the color here. This API's main clinet is input methods
so this shouldn't affect other editing operations, in particular, copy and paste, which uses HTMLConverter.
* editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt: Added.
* editing/mac/attributed-string/attribute-string-for-copy-with-color-filter.html: Added. Make sure the attributed string
generated for copy & paste does not invert foreground or background colors via textInputController.legacyAttributedString.
This is testing HTMLConverter, not editingAttributedStringFromRange, used by WebKit embedders and input methods.
* editing/mac/attributed-string/attributed-string-for-typing-with-color-filter-expected.txt:
* editing/mac/attributed-string/attributed-string-for-typing-with-color-filter.html: Added background color in the test.
* editing/style/set-backColor-with-color-filter-expected.txt: Added.
* editing/style/set-backColor-with-color-filter.html: Added.
* editing/style/set-foreColor-with-color-filter-expected.txt: Renamed from exec-command-foreColor-with-color-filter-expected.txt.
* editing/style/set-foreColor-with-color-filter.html: Renamed from exec-command-foreColor-with-color-filter.html.
Updated the test to use testRunner.execCommand which uses CommandFromMenuOrKeyBinding in applyCommandToFrame since we're
trying to test the code path taken by WebKit embedders and font panel here.

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

32 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/execCommand/set-backColor-with-color-filter-from-scripts-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/set-backColor-with-color-filter-from-scripts.html [new file with mode: 0644]
LayoutTests/editing/execCommand/set-foreColor-with-color-filter-from-scripts-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/set-foreColor-with-color-filter-from-scripts.html [moved from LayoutTests/editing/style/exec-command-foreColor-with-color-filter.html with 75% similarity]
LayoutTests/editing/mac/attributed-string/attrib-string-range-with-color-filter-expected.txt
LayoutTests/editing/mac/attributed-string/attrib-string-range-with-color-filter.html
LayoutTests/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt [new file with mode: 0644]
LayoutTests/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter.html [new file with mode: 0644]
LayoutTests/editing/mac/attributed-string/attributed-string-for-typing-with-color-filter-expected.txt
LayoutTests/editing/mac/attributed-string/attributed-string-for-typing-with-color-filter.html
LayoutTests/editing/style/set-backColor-with-color-filter-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/set-backColor-with-color-filter.html [new file with mode: 0644]
LayoutTests/editing/style/set-foreColor-with-color-filter-expected.txt [moved from LayoutTests/editing/style/exec-command-foreColor-with-color-filter-expected.txt with 58% similarity]
LayoutTests/editing/style/set-foreColor-with-color-filter.html [new file with mode: 0644]
LayoutTests/platform/mac-sierra/editing/mac/attributed-string/attrib-string-range-with-color-filter-expected.txt
LayoutTests/platform/mac-sierra/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-sierra/editing/mac/attributed-string/attributed-string-for-typing-with-color-filter-expected.txt
LayoutTests/platform/win/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/editing/EditingStyle.cpp
Source/WebCore/editing/EditingStyle.h
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/Editor.h
Source/WebCore/editing/EditorCommand.cpp
Source/WebCore/editing/cocoa/HTMLConverter.mm
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/WebView/WebHTMLView.mm
Tools/ChangeLog
Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl
Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp
Tools/WebKitTestRunner/InjectedBundle/TestRunner.h

index 204fc5f..6d28ff7 100644 (file)
@@ -1,3 +1,39 @@
+2018-07-20  Ryosuke Niwa  <rniwa@apple.com>
+
+        Picking a color from the color panel for typing attributes needs to inverse transform through color-filter
+        https://bugs.webkit.org/show_bug.cgi?id=187846
+
+        Reviewed by Simon Fraser.
+
+        Added tests to make sure foreground or background colors specified in execCommand don't get inverted.
+
+        Fixed the test which was asserting that textInputController.attributedSubstringFromRange doesn't invert the color
+        since that's exactly what WebKit embedders uses to compute the style for color pickers.
+
+        Added new tests for copy & paste case using textInputController.legacyAttributedString.
+
+        * editing/execCommand/set-backColor-with-color-filter-from-scripts-expected.txt: Added.
+        * editing/execCommand/set-backColor-with-color-filter-from-scripts.html: Added.
+        * editing/execCommand/set-foreColor-with-color-filter-from-scripts-expected.txt: Added.
+        * editing/execCommand/set-foreColor-with-color-filter-from-scripts.html: Added.
+        * editing/mac/attributed-string/attrib-string-range-with-color-filter-expected.txt:
+        * editing/mac/attributed-string/attrib-string-range-with-color-filter.html: This test uses attributedSubstringFromRange,
+        which implemented using editingAttributedStringFromRange in WebCore. Since this is exactly what WebKit embedders uses
+        to compute the current style of the selected text, we need to invert the color here. This API's main clinet is input methods
+        so this shouldn't affect other editing operations, in particular, copy and paste, which uses HTMLConverter.
+        * editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt: Added.
+        * editing/mac/attributed-string/attribute-string-for-copy-with-color-filter.html: Added. Make sure the attributed string
+        generated for copy & paste does not invert foreground or background colors via textInputController.legacyAttributedString.
+        This is testing HTMLConverter, not editingAttributedStringFromRange, used by WebKit embedders and input methods.
+        * editing/mac/attributed-string/attributed-string-for-typing-with-color-filter-expected.txt:
+        * editing/mac/attributed-string/attributed-string-for-typing-with-color-filter.html: Added background color in the test.
+        * editing/style/set-backColor-with-color-filter-expected.txt: Added.
+        * editing/style/set-backColor-with-color-filter.html: Added.
+        * editing/style/set-foreColor-with-color-filter-expected.txt: Renamed from exec-command-foreColor-with-color-filter-expected.txt.
+        * editing/style/set-foreColor-with-color-filter.html: Renamed from exec-command-foreColor-with-color-filter.html.
+        Updated the test to use testRunner.execCommand which uses CommandFromMenuOrKeyBinding in applyCommandToFrame since we're
+        trying to test the code path taken by WebKit embedders and font panel here.
+
 2018-07-20  Chris Dumez  <cdumez@apple.com>
 
         REGRESSION(PSON?): [ WK2 ] http/tests/workers/service/client-*-page-cache.html LayoutTests are flaky
diff --git a/LayoutTests/editing/execCommand/set-backColor-with-color-filter-from-scripts-expected.txt b/LayoutTests/editing/execCommand/set-backColor-with-color-filter-from-scripts-expected.txt
new file mode 100644 (file)
index 0000000..6a4c115
--- /dev/null
@@ -0,0 +1,6 @@
+Setting the background color should invert the color through -apple-color-filter
+
+"world" should be #224433 / rgb(34, 68, 51):
+| <span>
+|   style="background-color: rgb(34, 68, 51);"
+|   "<#selection-anchor>hello world<#selection-focus>"
diff --git a/LayoutTests/editing/execCommand/set-backColor-with-color-filter-from-scripts.html b/LayoutTests/editing/execCommand/set-backColor-with-color-filter-from-scripts.html
new file mode 100644 (file)
index 0000000..c55d44a
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
+<html>
+<body>
+<div id="test" style="-apple-color-filter: apple-invert-lightness()" contenteditable>hello world</div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+Markup.description('Setting the background color should invert the color through -apple-color-filter');
+
+window.getSelection().setPosition(test, 0);
+window.getSelection().modify('extend', 'forward', 'word');
+window.getSelection().modify('extend', 'forward', 'word');
+document.execCommand('backColor', false, '#224433');
+Markup.dump('test', `"world" should be #224433 / rgb(${0x22}, ${0x44}, ${0x33})`);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/execCommand/set-foreColor-with-color-filter-from-scripts-expected.txt b/LayoutTests/editing/execCommand/set-foreColor-with-color-filter-from-scripts-expected.txt
new file mode 100644 (file)
index 0000000..300cfbe
--- /dev/null
@@ -0,0 +1,6 @@
+Setting the foreground color should not invert the color through -apple-color-filter
+
+"world" should be #224433:
+| <font>
+|   color="#224433"
+|   "<#selection-anchor>hello world<#selection-focus>"
@@ -1,17 +1,16 @@
 <!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
 <html>
-<head>
-<title>Setting the foreground color should invert the color through -apple-color-filter</title>
-<script src="../../resources/dump-as-markup.js"></script>
-</head>
 <body>
 <div id="test" style="-apple-color-filter: apple-invert-lightness()" contenteditable>hello world</div>
+<script src="../../resources/dump-as-markup.js"></script>
 <script>
+Markup.description('Setting the foreground color should not invert the color through -apple-color-filter');
+
 window.getSelection().setPosition(test, 0);
 window.getSelection().modify('extend', 'forward', 'word');
 window.getSelection().modify('extend', 'forward', 'word');
 document.execCommand('foreColor', false, '#224433');
-Markup.dump('test', '"world" should be #ecfffd');
+Markup.dump('test', '"world" should be #224433');
 
 </script>
 </body>
index f151a69..e2526ba 100644 (file)
@@ -1,7 +1,7 @@
-Test that an NSAttributedString from a range doesn't convert colors through -apple-color-filter.
+Test that an NSAttributedString from a range converts colors through -apple-color-filter.
 
 [is t]
-    NSColor: #cccccc (sRGB)
+    NSColor: #5c5c5c (sRGB)
     NSFont: Times-Roman 16.00 pt.
 
 
index 81c8fcf..1c78820 100644 (file)
@@ -1,7 +1,7 @@
 <!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
 <div id="target" style="color: #CCC; -apple-color-filter: apple-invert-lightness();"contenteditable>This text is light gray</div>
 <p>
-    Test that an NSAttributedString from a range doesn't convert colors through -apple-color-filter.
+    Test that an NSAttributedString from a range converts colors through -apple-color-filter.
 </p>
 <script src="resources/dump-attributed-string.js"></script>
 <script>
diff --git a/LayoutTests/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt b/LayoutTests/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt
new file mode 100644 (file)
index 0000000..be88a96
--- /dev/null
@@ -0,0 +1,32 @@
+Test that an NSAttributedString for copy doesn't convert colors through -apple-color-filter.
+
+NSParagraphStyle:
+Alignment 4
+    LineSpacing: 0
+    ParagraphSpacing: 0
+    ParagraphSpacingBefore: 0
+    HeadIndent: 0
+    TailIndent: 0
+    FirstLineHeadIndent: 0
+    LineHeight: 0/0
+    LineHeightMultiple: 0
+    LineBreakMode: 0
+    Tabs: ()
+    DefaultTabInterval: 36
+    Blocks: (
+)
+    Lists: (
+)
+    BaseWritingDirection: 0
+    HyphenationFactor: 0
+    TighteningForTruncation: YES
+    HeaderLevel: 0
+[is]
+    NSBackgroundColor: #336699 (sRGB)
+    NSColor: #cccccc (sRGB)
+    NSFont: Times-Roman 16.00 pt.
+    NSKern: 0pt
+    NSStrokeColor: #cccccc (sRGB)
+    NSStrokeWidth: 0
+
+
diff --git a/LayoutTests/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter.html b/LayoutTests/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter.html
new file mode 100644 (file)
index 0000000..a7b574d
--- /dev/null
@@ -0,0 +1,34 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
+<div id="target" style="color: #ccc; background: #369; -apple-color-filter: apple-invert-lightness();" contenteditable>This text is light gray</div>
+<p>
+    Test that an NSAttributedString for copy doesn't convert colors through -apple-color-filter.
+</p>
+<script src="resources/dump-attributed-string.js"></script>
+<script>
+    var shouldAutoDump = false;
+</script>
+<pre id="console"></pre>
+<script>
+    function log(message)
+    {
+        document.getElementById("console").append(message + "\n");
+    }
+
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+
+        var target = document.getElementById("target");
+        target.focus();
+
+        const range = new Range;
+        range.setStart(target.firstChild, 2);
+        range.setEnd(target.firstChild, 4);
+
+        var attributedString = textInputController.legacyAttributedString(range);
+        var serializedString = serializeAttributedString(attributedString);
+        log(serializedString);
+
+        target.parentNode.removeChild(target);
+    } else
+        log("This test can only run in DumpRenderTree.");
+</script>
index 1a5e437..ac8cd04 100644 (file)
@@ -1,9 +1,10 @@
 Some text here
 Input:
-<div id="editor" style="-apple-color-filter: apple-invert-lightness(); color: rgba(20, 20, 20, 0.4);" contenteditable="">Some text here</div>
+<div id="editor" style="-apple-color-filter: apple-invert-lightness(); color: rgba(20, 20, 20, 0.4); background: #ccc;" contenteditable="">Some text here</div>
 
 Output:
 [ ]
+    NSBackgroundColor: #5c5c5c (sRGB)
     NSColor: rgba(239, 239, 239, 0.4) (sRGB)
     NSFont: Times-Roman 16.00 pt.
 
index 99a9b3c..4b0a8ac 100644 (file)
@@ -8,7 +8,7 @@
 </head>
 <body>
 
-<div id="editor" style="-apple-color-filter: apple-invert-lightness(); color: rgba(20, 20, 20, 0.4);" contenteditable>Some text here</div>
+<div id="editor" style="-apple-color-filter: apple-invert-lightness(); color: rgba(20, 20, 20, 0.4); background: #ccc;" contenteditable>Some text here</div>
 <pre id="result">This test requires DumpRenderTree</pre>
 <script type="text/javascript">
     
diff --git a/LayoutTests/editing/style/set-backColor-with-color-filter-expected.txt b/LayoutTests/editing/style/set-backColor-with-color-filter-expected.txt
new file mode 100644 (file)
index 0000000..6b26cf3
--- /dev/null
@@ -0,0 +1,6 @@
+Setting the background color should invert the color through -apple-color-filter
+
+"world" should be #ecfffd / rgb(236, 255, 253):
+| <span>
+|   style="background-color: rgb(236, 255, 253);"
+|   "<#selection-anchor>hello world<#selection-focus>"
diff --git a/LayoutTests/editing/style/set-backColor-with-color-filter.html b/LayoutTests/editing/style/set-backColor-with-color-filter.html
new file mode 100644 (file)
index 0000000..c9e9e6b
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
+<html>
+<body>
+<div id="test" style="-apple-color-filter: apple-invert-lightness()" contenteditable>hello world</div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('Setting the background color should invert the color through -apple-color-filter');
+
+window.getSelection().setPosition(test, 0);
+window.getSelection().modify('extend', 'forward', 'word');
+window.getSelection().modify('extend', 'forward', 'word');
+if (window.testRunner)
+    testRunner.execCommand('backColor', false, '#224433');
+else
+    document.write('This test requires WebKitTestRunner or DumpRenderTree');
+
+Markup.dump('test', `"world" should be #ecfffd / rgb(${0xec}, ${0xff}, ${0xfd})`);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/style/set-foreColor-with-color-filter.html b/LayoutTests/editing/style/set-foreColor-with-color-filter.html
new file mode 100644 (file)
index 0000000..542c53e
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
+<html>
+<body>
+<div id="test" style="-apple-color-filter: apple-invert-lightness()" contenteditable>hello world</div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('Setting the foreground color should invert the color through -apple-color-filter');
+
+window.getSelection().setPosition(test, 0);
+window.getSelection().modify('extend', 'forward', 'word');
+window.getSelection().modify('extend', 'forward', 'word');
+if (window.testRunner)
+    testRunner.execCommand('foreColor', false, '#224433');
+else
+    document.write('This test requires WebKitTestRunner or DumpRenderTree');
+
+Markup.dump('test', '"world" should be #ecfffd');
+
+</script>
+</body>
+</html>
index fab076c..8939a6f 100644 (file)
@@ -1,7 +1,7 @@
-Test that an NSAttributedString from a range doesn't convert colors through -apple-color-filter.
+Test that an NSAttributedString from a range converts colors through -apple-color-filter.
 
 [is t]
-    NSColor: #cccccc (NSCustomColorSpace)
+    NSColor: #5c5c5c (NSCustomColorSpace)
     NSFont: Times-Roman 16.00 pt.
 
 
diff --git a/LayoutTests/platform/mac-sierra/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt b/LayoutTests/platform/mac-sierra/editing/mac/attributed-string/attribute-string-for-copy-with-color-filter-expected.txt
new file mode 100644 (file)
index 0000000..2c430e9
--- /dev/null
@@ -0,0 +1,32 @@
+Test that an NSAttributedString for copy doesn't convert colors through -apple-color-filter.
+
+NSParagraphStyle:
+Alignment 4
+    LineSpacing: 0
+    ParagraphSpacing: 0
+    ParagraphSpacingBefore: 0
+    HeadIndent: 0
+    TailIndent: 0
+    FirstLineHeadIndent: 0
+    LineHeight: 0/0
+    LineHeightMultiple: 0
+    LineBreakMode: 0
+    Tabs: ()
+    DefaultTabInterval: 36
+    Blocks: (
+)
+    Lists: (
+)
+    BaseWritingDirection: 0
+    HyphenationFactor: 0
+    TighteningForTruncation: YES
+    HeaderLevel: 0
+[is]
+    NSBackgroundColor: #336699 (NSCustomColorSpace)
+    NSColor: #cccccc (NSCustomColorSpace)
+    NSFont: Times-Roman 16.00 pt.
+    NSKern: 0pt
+    NSStrokeColor: #cccccc (NSCustomColorSpace)
+    NSStrokeWidth: 0
+
+
index 5824c77..196d494 100644 (file)
@@ -1,9 +1,10 @@
 Some text here
 Input:
-<div id="editor" style="-apple-color-filter: apple-invert-lightness(); color: rgba(20, 20, 20, 0.4);" contenteditable="">Some text here</div>
+<div id="editor" style="-apple-color-filter: apple-invert-lightness(); color: rgba(20, 20, 20, 0.4); background: #ccc;" contenteditable="">Some text here</div>
 
 Output:
 [ ]
+    NSBackgroundColor: #5c5c5c (NSCustomColorSpace)
     NSColor: rgba(239, 239, 239, 0.4) (NSCustomColorSpace)
     NSFont: Times-Roman 16.00 pt.
 
index 2a01fdf..06c9bb3 100644 (file)
@@ -98,7 +98,8 @@ webkit.org/b/173281 http/tests/security/mixedContent/insecure-image-redirects-to
 webkit.org/b/173281 http/tests/security/mixedContent/secure-redirect-to-insecure-redirect-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html [ Skip ]
 webkit.org/b/173281 http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-insecure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html [ Skip ]
 webkit.org/b/173281 http/tests/security/mixedContent/secure-redirect-to-secure-redirect-to-basic-auth-secure-image-allowCrossOriginSubresourcesToAskForCredentials.https.html [ Skip ]
-webkit.org/b/173281 editing/style/exec-command-foreColor-with-color-filter.html [ Skip ]
+webkit.org/b/173281 editing/style/set-foreColor-with-color-filter.html [ Skip ]
+webkit.org/b/173281 editing/style/set-backColor-with-color-filter.html [ Skip ]
 
 # TODO HW filters not yet supported on Windows
 webkit.org/b/74716 css3/filters/effect-blur-hw.html [ Skip ]
index 485ee93..5fc609f 100644 (file)
@@ -1,3 +1,51 @@
+2018-07-20  Ryosuke Niwa  <rniwa@apple.com>
+
+        Picking a color from the color panel for typing attributes needs to inverse transform through color-filter
+        https://bugs.webkit.org/show_bug.cgi?id=187846
+
+        Reviewed by Simon Fraser.
+
+        This patch refines the color inversion for editing introduced in r234005 so that font panels and WebKit embedders
+        can get and set inverted colors using attributed strings for a selected text in an editable region.
+
+        More specifically, when font panels or WebKit embedders set a font color or a background color via WebView's
+        _applyStyleToSelection and _applyEditingStyleToSelection, WebKit would automatically invert the color before inserting
+        into DOM so that the color visible to the user matches that's given to WebKit. Conversely,
+        [WebView attributedSubstringFromRange:nsRange] now return the color visible to the user, i.e. the color after
+        the color filter had been applied, so that some WebKit embedders can present the visually identical color to the user.
+
+        Because DOM never sees the color filter's effect in the computed style, etc... this patch reverts the change made to
+        StyleChange in r234005 to avoid inverting the color passed to execCommand. This makes editing apps which is unaware
+        of the dark mode or -apple-color-filter continue to function (because the color picker implemented in DOM will be
+        applied of the same color filter before being presented to the user).
+
+        Finally, this patch introduces a testing hook in applyCommandToFrame so that executing foreColor or backColor with
+        the soruce of CommandFromMenuOrKeyBinding would trigger the same code path as the one taken by Objective-C 
+
+        Tests: editing/execCommand/set-backColor-with-color-filter-from-scripts.html
+               editing/execCommand/set-foreColor-with-color-filter-from-scripts.html
+               editing/mac/attributed-string/attribute-string-for-copy-with-color-filter.html
+               editing/style/set-backColor-with-color-filter.html
+               editing/style/set-foreColor-with-color-filter.html
+
+        * editing/EditingStyle.cpp:
+        (WebCore::EditingStyle::inverseTransformColorIfNeeded): Added.
+        (WebCore::StyleChange::StyleChange): Revert the change made in r234005 since this code is also used by execCommand
+        which is not desirable, and won't work for background color.
+        (WebCore::StyleChange::extractTextStyles): Ditto.
+        * editing/EditingStyle.h:
+        * editing/Editor.cpp:
+        (WebCore::Editor::applyStyle):
+        (WebCore::Editor::applyStyleToSelection): Call EditingStyle::inverseTransformColorIfNeeded when ColorFilterMode is
+        set to InvertColor.
+        * editing/Editor.h:
+        * editing/EditorCommand.cpp:
+        (WebCore::applyCommandToFrame): Added the aforementioned testing hook.
+        * editing/cocoa/HTMLConverter.mm:
+        (WebCore::editingAttributedStringFromRange): Take the color filtr into account. Some WebKit embedders use this
+        function to compute the font color in the selected text. Note that this function is mostly used for input methods
+        so the color doesn't really matter, and its implementation is distinct from that of HTMLConverter.
+
 2018-07-19  Jer Noble  <jer.noble@apple.com>
 
         HLS resources with remote subresources will not taint canvasses.
index 7afde63..dbf60e6 100644 (file)
@@ -1538,6 +1538,36 @@ WritingDirection EditingStyle::textDirectionForSelection(const VisibleSelection&
     return foundDirection;
 }
 
+Ref<EditingStyle> EditingStyle::inverseTransformColorIfNeeded(Element& element)
+{
+    auto* renderer = element.renderer();
+    if (!m_mutableStyle || !renderer || !renderer->style().hasAppleColorFilter())
+        return *this;
+
+    bool hasColor = m_mutableStyle->getPropertyCSSValue(CSSPropertyColor);
+    bool hasBackgroundColor = m_mutableStyle->getPropertyCSSValue(CSSPropertyBackgroundColor);
+    if (!hasColor && !hasBackgroundColor)
+        return *this;
+
+    auto styleWithInvertedColors = copy();
+    ASSERT(styleWithInvertedColors->m_mutableStyle);
+
+    const auto& colorFilter = renderer->style().appleColorFilter();
+    auto invertedColor = [&](CSSPropertyID propertyID) {
+        Color newColor = cssValueToColor(extractPropertyValue(*m_mutableStyle, propertyID).get());
+        colorFilter.inverseTransformColor(newColor);
+        styleWithInvertedColors->m_mutableStyle->setProperty(propertyID, newColor.cssText());
+    };
+
+    if (hasColor)
+        invertedColor(CSSPropertyColor);
+
+    if (hasBackgroundColor)
+        invertedColor(CSSPropertyBackgroundColor);
+
+    return styleWithInvertedColors;
+}
+
 static void reconcileTextDecorationProperties(MutableStyleProperties* style)
 {    
     RefPtr<CSSValue> textDecorationsInEffect = style->getPropertyCSSValue(CSSPropertyWebkitTextDecorationsInEffect);
@@ -1580,7 +1610,7 @@ StyleChange::StyleChange(EditingStyle* style, const Position& position)
     reconcileTextDecorationProperties(mutableStyle.get());
     bool shouldStyleWithCSS = document->frame()->editor().shouldStyleWithCSS();
     if (!shouldStyleWithCSS)
-        extractTextStyles(document, *node, *mutableStyle, computedStyle.useFixedFontDefaultSize());
+        extractTextStyles(document, *mutableStyle, computedStyle.useFixedFontDefaultSize());
 
     bool shouldAddUnderline = style->underlineChange() == TextDecorationChange::Add;
     bool shouldAddStrikeThrough = style->strikeThroughChange() == TextDecorationChange::Add;
@@ -1653,7 +1683,7 @@ static void setTextDecorationProperty(MutableStyleProperties& style, const CSSVa
     }
 }
 
-void StyleChange::extractTextStyles(Document* document, Node& startNode, MutableStyleProperties& style, bool shouldUseFixedFontDefaultSize)
+void StyleChange::extractTextStyles(Document* document, MutableStyleProperties& style, bool shouldUseFixedFontDefaultSize)
 {
     if (identifierForStyleProperty(style, CSSPropertyFontWeight) == CSSValueBold) {
         style.removeProperty(CSSPropertyFontWeight);
@@ -1697,12 +1727,7 @@ void StyleChange::extractTextStyles(Document* document, Node& startNode, Mutable
     }
 
     if (style.getPropertyCSSValue(CSSPropertyColor)) {
-        Color newColor = textColorFromStyle(style);
-
-        if (startNode.renderer() && startNode.renderer()->style().hasAppleColorFilter())
-            startNode.renderer()->style().appleColorFilter().inverseTransformColor(newColor);
-
-        m_applyFontColor = newColor.serialized();
+        m_applyFontColor = Color(textColorFromStyle(style)).serialized();
         style.removeProperty(CSSPropertyColor);
     }
 
index 8a8326a..dc2978c 100644 (file)
@@ -168,6 +168,8 @@ public:
     WEBCORE_EXPORT static RefPtr<EditingStyle> styleAtSelectionStart(const VisibleSelection&, bool shouldUseBackgroundColorInEffect = false);
     static WritingDirection textDirectionForSelection(const VisibleSelection&, EditingStyle* typingStyle, bool& hasNestedOrMultipleEmbeddings);
 
+    Ref<EditingStyle> inverseTransformColorIfNeeded(Element&);
+
 private:
     EditingStyle();
     EditingStyle(Node*, PropertiesToInclude);
@@ -222,7 +224,7 @@ public:
         return !(*this == other);
     }
 private:
-    void extractTextStyles(Document*, Node& startNode, MutableStyleProperties&, bool shouldUseFixedFontDefaultSize);
+    void extractTextStyles(Document*, MutableStyleProperties&, bool shouldUseFixedFontDefaultSize);
 
     RefPtr<MutableStyleProperties> m_cssStyle;
     bool m_applyBold = false;
index 1d9d991..27c2b9e 100644 (file)
@@ -892,10 +892,10 @@ Element* Editor::findEventTargetFromSelection() const
 void Editor::applyStyle(StyleProperties* style, EditAction editingAction)
 {
     if (style)
-        applyStyle(EditingStyle::create(style), editingAction);
+        applyStyle(EditingStyle::create(style), editingAction, ColorFilterMode::UseOriginalColor);
 }
 
-void Editor::applyStyle(RefPtr<EditingStyle>&& style, EditAction editingAction)
+void Editor::applyStyle(RefPtr<EditingStyle>&& style, EditAction editingAction, ColorFilterMode colorFilterMode)
 {
     if (!style)
         return;
@@ -910,12 +910,14 @@ void Editor::applyStyle(RefPtr<EditingStyle>&& style, EditAction editingAction)
     if (element && !dispatchBeforeInputEvent(*element, inputTypeName, inputEventData))
         return;
 
+    Ref<EditingStyle> styleToApply = colorFilterMode == ColorFilterMode::InvertColor ? style->inverseTransformColorIfNeeded(*element) : style.releaseNonNull();
+
     switch (selectionType) {
     case VisibleSelection::CaretSelection:
-        computeAndSetTypingStyle(*style, editingAction);
+        computeAndSetTypingStyle(styleToApply.get(), editingAction);
         break;
     case VisibleSelection::RangeSelection:
-        ApplyStyleCommand::create(document(), style.get(), editingAction)->apply();
+        ApplyStyleCommand::create(document(), styleToApply.ptr(), editingAction)->apply();
         break;
     default:
         break;
@@ -962,7 +964,7 @@ void Editor::applyStyleToSelection(StyleProperties* style, EditAction editingAct
     applyStyle(style, editingAction);
 }
 
-void Editor::applyStyleToSelection(Ref<EditingStyle>&& style, EditAction editingAction)
+void Editor::applyStyleToSelection(Ref<EditingStyle>&& style, EditAction editingAction, ColorFilterMode colorFilterMode)
 {
     if (style->isEmpty() || !canEditRichly())
         return;
@@ -971,7 +973,7 @@ void Editor::applyStyleToSelection(Ref<EditingStyle>&& style, EditAction editing
     if (!client() || !client()->shouldApplyStyle(style->styleWithResolvedTextDecorations().ptr(), m_frame.selection().toNormalizedRange().get()))
         return;
 
-    applyStyle(WTFMove(style), editingAction);
+    applyStyle(WTFMove(style), editingAction, colorFilterMode);
 }
 
 void Editor::applyParagraphStyleToSelection(StyleProperties* style, EditAction editingAction)
index c594bbd..17a86f2 100644 (file)
@@ -216,10 +216,11 @@ public:
 #endif
 
     WEBCORE_EXPORT void applyStyle(StyleProperties*, EditAction = EditActionUnspecified);
-    void applyStyle(RefPtr<EditingStyle>&&, EditAction);
+    enum class ColorFilterMode { InvertColor, UseOriginalColor };
+    void applyStyle(RefPtr<EditingStyle>&&, EditAction, ColorFilterMode);
     void applyParagraphStyle(StyleProperties*, EditAction = EditActionUnspecified);
     WEBCORE_EXPORT void applyStyleToSelection(StyleProperties*, EditAction);
-    WEBCORE_EXPORT void applyStyleToSelection(Ref<EditingStyle>&&, EditAction);
+    WEBCORE_EXPORT void applyStyleToSelection(Ref<EditingStyle>&&, EditAction, ColorFilterMode);
     void applyParagraphStyleToSelection(StyleProperties*, EditAction);
 
     // Returns whether or not we should proceed with editing.
index 8a5021f..4c93d4a 100644 (file)
@@ -100,11 +100,12 @@ static bool applyCommandToFrame(Frame& frame, EditorCommandSource source, EditAc
     // FIXME: We don't call shouldApplyStyle when the source is DOM; is there a good reason for that?
     switch (source) {
     case CommandFromMenuOrKeyBinding:
-        frame.editor().applyStyleToSelection(WTFMove(style), action);
+        // Use InvertColor for testing purposes. foreColor and backColor are never triggered with CommandFromMenuOrKeyBinding outside DRT/WTR.
+        frame.editor().applyStyleToSelection(WTFMove(style), action, Editor::ColorFilterMode::InvertColor);
         return true;
     case CommandFromDOM:
     case CommandFromDOMWithUserInterface:
-        frame.editor().applyStyle(WTFMove(style), EditActionUnspecified);
+        frame.editor().applyStyle(WTFMove(style), EditActionUnspecified, Editor::ColorFilterMode::UseOriginalColor);
         return true;
     }
     ASSERT_NOT_REACHED();
index 3907235..527d992 100644 (file)
@@ -2514,13 +2514,13 @@ NSAttributedString *editingAttributedStringFromRange(Range& range, IncludeImages
         else
             [attrs.get() setObject:[fontManager convertFont:WebDefaultFont() toSize:style.fontCascade().primaryFont().platformData().size()] forKey:NSFontAttributeName];
 
-        Color foregroundColor = style.visitedDependentColor(CSSPropertyColor);
+        Color foregroundColor = style.visitedDependentColorWithColorFilter(CSSPropertyColor);
         if (foregroundColor.isVisible())
             [attrs.get() setObject:nsColor(foregroundColor) forKey:NSForegroundColorAttributeName];
         else
             [attrs.get() removeObjectForKey:NSForegroundColorAttributeName];
 
-        Color backgroundColor = style.visitedDependentColor(CSSPropertyBackgroundColor);
+        Color backgroundColor = style.visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor);
         if (backgroundColor.isVisible())
             [attrs.get() setObject:nsColor(backgroundColor) forKey:NSBackgroundColorAttributeName];
         else
index a069a3f..f0f482e 100644 (file)
@@ -1,3 +1,16 @@
+2018-07-20  Ryosuke Niwa  <rniwa@apple.com>
+
+        Picking a color from the color panel for typing attributes needs to inverse transform through color-filter
+        https://bugs.webkit.org/show_bug.cgi?id=187846
+
+        Reviewed by Simon Fraser.
+
+        Invert the filtered font and background colors when using font panels, font pasteboard, and other Objective-C APIs.
+
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView _applyStyleToSelection:withUndoAction:]): Share code with _applyEditingStyleToSelection.
+        (-[WebHTMLView _applyEditingStyleToSelection:withUndoAction:]):
+
 2018-07-11  Dean Jackson  <dino@apple.com>
 
         Allow removal of white backgrounds
index e22182c..f52a48c 100644 (file)
@@ -5204,17 +5204,13 @@ static RefPtr<KeyboardEvent> currentKeyboardEvent(Frame* coreFrame)
 
 - (void)_applyStyleToSelection:(DOMCSSStyleDeclaration *)style withUndoAction:(EditAction)undoAction
 {
-    if (Frame* coreFrame = core([self _frame])) {
-        // FIXME: We shouldn't have to make a copy here. We want callers of this function to work directly with StyleProperties eventually.
-        Ref<MutableStyleProperties> properties(core(style)->copyProperties());
-        coreFrame->editor().applyStyleToSelection(properties.ptr(), undoAction);
-    }
+    [self _applyEditingStyleToSelection:EditingStyle::create(core(style)) withUndoAction:undoAction];
 }
 
 - (void)_applyEditingStyleToSelection:(Ref<EditingStyle>&&)editingStyle withUndoAction:(EditAction)undoAction
 {
     if (Frame* coreFrame = core([self _frame]))
-        coreFrame->editor().applyStyleToSelection(WTFMove(editingStyle), undoAction);
+        coreFrame->editor().applyStyleToSelection(WTFMove(editingStyle), undoAction, Editor::ColorFilterMode::InvertColor);
 }
 
 #if PLATFORM(MAC)
index 8c83ccf..58dfe0b 100644 (file)
@@ -1,3 +1,20 @@
+2018-07-20  Ryosuke Niwa  <rniwa@apple.com>
+
+        Picking a color from the color panel for typing attributes needs to inverse transform through color-filter
+        https://bugs.webkit.org/show_bug.cgi?id=187846
+
+        Reviewed by Simon Fraser.
+
+        Fixed the bug that testRunner.execCommand was using the second argument as the value.
+
+        The second argument, aShowDefaultUI, should always be ignored in testRunner.execCommand,
+        and the third argument should be used as the value. DumpRenderTree's implementation does this already.
+
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::execCommand):
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+
 2018-07-20  Dean Jackson  <dino@apple.com>
 
         Accidentally checked in the wrong version of these files.
index c09cbae..e479a77 100644 (file)
@@ -97,7 +97,7 @@ interface TestRunner {
 
     // Special DOM functions.
     void clearBackForwardList();
-    void execCommand(DOMString name, DOMString argument);
+    void execCommand(DOMString name, DOMString showUI, DOMString value);
     boolean isCommandEnabled(DOMString name);
     unsigned long windowCount();
 
index 374ec79..276133f 100644 (file)
@@ -291,9 +291,9 @@ void TestRunner::keepWebHistory()
     InjectedBundle::singleton().postSetAddsVisitedLinks(true);
 }
 
-void TestRunner::execCommand(JSStringRef name, JSStringRef argument)
+void TestRunner::execCommand(JSStringRef name, JSStringRef showUI, JSStringRef value)
 {
-    WKBundlePageExecuteEditingCommand(InjectedBundle::singleton().page()->page(), toWK(name).get(), toWK(argument).get());
+    WKBundlePageExecuteEditingCommand(InjectedBundle::singleton().page()->page(), toWK(name).get(), toWK(value).get());
 }
 
 bool TestRunner::findString(JSStringRef target, JSValueRef optionsArrayAsValue)
index 4dad0f0..aae78fd 100644 (file)
@@ -135,7 +135,7 @@ public:
 
     // Special DOM functions.
     void clearBackForwardList();
-    void execCommand(JSStringRef name, JSStringRef argument);
+    void execCommand(JSStringRef name, JSStringRef showUI, JSStringRef value);
     bool isCommandEnabled(JSStringRef name);
     unsigned windowCount();