Spelling dots are drawn in the wrong place
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Nov 2018 00:09:08 +0000 (00:09 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Nov 2018 00:09:08 +0000 (00:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190764

Reviewed by Dean Jackson.

Source/WebCore:

- Dots should not be clipped.
- Dots should be horizontally centered.
- Dots should be drawn behind the text.
- Distance from the baseline to the top of the dot should be 11.035% of font size.
- Dot diameter should be 13.247% of the font size.
- Distance between the dots (right side of the left dot to left side of the right dot) should be 9.457% of the font size.
- The "font size" used in these calculations should be clamped so it's 10px <= font size <= 40px.

Tests: editing/spelling/spelling-dots-position-2.html
       editing/spelling/spelling-dots-position-3.html
       editing/spelling/spelling-dots-position.html
       editing/spelling/spelling-dots-repaint.html

* platform/graphics/cocoa/GraphicsContextCocoa.mm:
(WebCore::colorForMarkerLineStyle): Align iOS and macOS implementations.
(WebCore::GraphicsContext::drawDotsForDocumentMarker): Place the dots correctly.
* rendering/InlineFlowBox.cpp:
(WebCore::InlineFlowBox::addToLine): The KnownToHaveNoOverflow flag should be cleared if the element has spelling dots,
    because there is no guarantee the spelling dots will lie inside the layout rect of the element.
(WebCore::InlineFlowBox::addTextBoxVisualOverflow): Update the repaint rects to include splling dot positions.
* rendering/InlineFlowBox.h: Comments should explain why, not say what.
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paint): Draw the dots behind the text.
(WebCore::InlineTextBox::hasMarkers const): Convenience.
(WebCore::InlineTextBox::paintPlatformDocumentMarkers): Refactor bounds information into a helper function.
(WebCore::InlineTextBox::calculateUnionOfAllDocumentMarkerBounds const): Use for repaint rect calculation.
(WebCore::InlineTextBox::calculateDocumentMarkerBounds const): Place the dots correctly.
(WebCore::InlineTextBox::paintPlatformDocumentMarker): Call the helper method.
(WebCore::InlineTextBox::collectMarkedTextsForDocumentMarkers const):
(WebCore::InlineTextBox::collectMarkedTextsForDocumentMarkers): Deleted.
* rendering/InlineTextBox.h: Declare the helper methods.
* rendering/SimpleLineLayout.cpp: Simple line layout doesn't know how to paint spelling dots, so make the presence of
    spelling dots opt us out of SLL.
(WebCore::SimpleLineLayout::canUseForWithReason):
* rendering/SimpleLineLayoutCoverage.cpp:
(WebCore::SimpleLineLayout::printReason):
* rendering/SimpleLineLayoutCoverage.h: Add a new opt-out reason.

Tools:

Previously, it was impossible for WebKitTestRunner to draw spelling dots. This patch adds support for a header
at the top of test files, of the form <!-- webkit-test-runner [ spellCheckingDots=true ] --> which will cause
dots to be drawn.

* WebKitTestRunner/TestController.cpp:
(WTR::TestController::resetStateToConsistentValues):
(WTR::updateTestOptionsFromTestHeader):
(WTR::TestController::platformResetStateToConsistentValues):
* WebKitTestRunner/TestController.h:
* WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::hasSameInitializationOptions const):
* WebKitTestRunner/cocoa/TestControllerCocoa.mm:
(WTR::TestController::cocoaResetStateToConsistentValues):
* WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
* WebKitTestRunner/ios/TestControllerIOS.mm:
(WTR::TestController::platformResetStateToConsistentValues):
* WebKitTestRunner/mac/TestControllerMac.mm:
(WTR::TestController::platformResetStateToConsistentValues):

LayoutTests:

* editing/spelling/resources/VerySmallDescentAhem.ttf: Added. In order to test repaint rects,
      this is a version of Ahem with a very small descent, so that the dots don't intersect
      with the text itself.
* editing/spelling/spelling-dots-position-2-expected-mismatch.html: Added.
* editing/spelling/spelling-dots-position-2.html: Added. Make sure dots are painted in the
      correct vertical place.
* editing/spelling/spelling-dots-position-3-expected-mismatch.html: Added.
* editing/spelling/spelling-dots-position-3.html: Added. Make sure dots are painted in the
      correct vertical place.
* editing/spelling/spelling-dots-position-expected.html: Added.
* editing/spelling/spelling-dots-position.html: Added. Make sure dots are not painted in
      the wrong place.
* editing/spelling/spelling-dots-repaint-expected.html: Added.
* editing/spelling/spelling-dots-repaint.html: Added. Test repaint by drawing an element
      with spelling dots, and then deleting the element from the document. The spelling
      dots should be removed too.
* fast/writing-mode/english-bt-text-with-spelling-marker-expected.html:
* fast/writing-mode/english-bt-text-with-spelling-marker.html: Update the test to compensate
      for new spelling dot positions.

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

29 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/spelling/resources/VerySmallDescentAhem.ttf [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-position-2-expected-mismatch.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-position-2.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-position-3-expected-mismatch.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-position-3.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-position-expected.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-position.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-repaint-expected.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-dots-repaint.html [new file with mode: 0644]
LayoutTests/fast/writing-mode/english-bt-text-with-spelling-marker-expected.html
LayoutTests/fast/writing-mode/english-bt-text-with-spelling-marker.html
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm
Source/WebCore/rendering/InlineFlowBox.cpp
Source/WebCore/rendering/InlineFlowBox.h
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/InlineTextBox.h
Source/WebCore/rendering/SimpleLineLayout.cpp
Source/WebCore/rendering/SimpleLineLayoutCoverage.cpp
Source/WebCore/rendering/SimpleLineLayoutCoverage.h
Tools/ChangeLog
Tools/WebKitTestRunner/TestController.cpp
Tools/WebKitTestRunner/TestController.h
Tools/WebKitTestRunner/TestOptions.h
Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm
Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
Tools/WebKitTestRunner/ios/TestControllerIOS.mm
Tools/WebKitTestRunner/mac/TestControllerMac.mm

index e6d45be..7be73b9 100644 (file)
@@ -1,3 +1,30 @@
+2018-11-05  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Spelling dots are drawn in the wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=190764
+
+        Reviewed by Dean Jackson.
+
+        * editing/spelling/resources/VerySmallDescentAhem.ttf: Added. In order to test repaint rects,
+              this is a version of Ahem with a very small descent, so that the dots don't intersect
+              with the text itself.
+        * editing/spelling/spelling-dots-position-2-expected-mismatch.html: Added.
+        * editing/spelling/spelling-dots-position-2.html: Added. Make sure dots are painted in the
+              correct vertical place.
+        * editing/spelling/spelling-dots-position-3-expected-mismatch.html: Added.
+        * editing/spelling/spelling-dots-position-3.html: Added. Make sure dots are painted in the
+              correct vertical place.
+        * editing/spelling/spelling-dots-position-expected.html: Added.
+        * editing/spelling/spelling-dots-position.html: Added. Make sure dots are not painted in
+              the wrong place.
+        * editing/spelling/spelling-dots-repaint-expected.html: Added.
+        * editing/spelling/spelling-dots-repaint.html: Added. Test repaint by drawing an element
+              with spelling dots, and then deleting the element from the document. The spelling
+              dots should be removed too.
+        * fast/writing-mode/english-bt-text-with-spelling-marker-expected.html:
+        * fast/writing-mode/english-bt-text-with-spelling-marker.html: Update the test to compensate
+              for new spelling dot positions.
+
 2018-11-05  Ryan Haddad  <ryanhaddad@apple.com>
 
         Layout Test imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-setRemoteDescription-tracks.https.html is flaky
diff --git a/LayoutTests/editing/spelling/resources/VerySmallDescentAhem.ttf b/LayoutTests/editing/spelling/resources/VerySmallDescentAhem.ttf
new file mode 100644 (file)
index 0000000..9093b79
Binary files /dev/null and b/LayoutTests/editing/spelling/resources/VerySmallDescentAhem.ttf differ
diff --git a/LayoutTests/editing/spelling/spelling-dots-position-2-expected-mismatch.html b/LayoutTests/editing/spelling/spelling-dots-position-2-expected-mismatch.html
new file mode 100644 (file)
index 0000000..2655f4e
--- /dev/null
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-dots-position-2.html b/LayoutTests/editing/spelling/spelling-dots-position-2.html
new file mode 100644 (file)
index 0000000..f787f9d
--- /dev/null
@@ -0,0 +1,67 @@
+<!DOCTYPE html><!-- webkit-test-runner [ spellCheckingDots=true ] -->
+<html>
+<head>
+
+<style>
+.editing {
+    font-family: 'Times New Roman';
+    font-size: 36px;
+    color: transparent;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+function editingTest() {
+    typeCharacterCommand('w');
+    typeCharacterCommand('a');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('e');
+    typeCharacterCommand('r');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('d');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('a');
+    typeCharacterCommand(' ');
+    let ascentSetter = document.createElement("div");
+    ascentSetter.style.width = "1px";
+    ascentSetter.style.height = "150px";
+    ascentSetter.style.display = "inline-block";
+    let root = document.getElementById("root");
+    root.appendChild(ascentSetter);
+    let baseline = ascentSetter.offsetTop + ascentSetter.offsetHeight;
+    let left = root.offsetLeft;
+    let width = ascentSetter.offsetLeft - left;
+    let fontSize = Number.parseInt(window.getComputedStyle(root).getPropertyValue("font-size"));
+    let blocker = document.createElement("div");
+    blocker.style.background = "white";
+    blocker.style.position = "absolute";
+    blocker.style.left = `${left}px`;
+    blocker.style.top = `${baseline + 1 + fontSize * 0.11035}px`;
+    blocker.style.width = `${width}px`;
+    blocker.style.height = `${fontSize * 0.13247}px`;
+    document.body.appendChild(blocker);
+    root.blur();
+}
+
+</script>
+
+<title>Editing Test</title>
+</head>
+<body>
+<div contenteditable id="root" class="editing">
+<span id="test"></span>
+</div>
+
+<script>
+    if (window.internals)
+        internals.setContinuousSpellCheckingEnabled(true);
+    runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-dots-position-3-expected-mismatch.html b/LayoutTests/editing/spelling/spelling-dots-position-3-expected-mismatch.html
new file mode 100644 (file)
index 0000000..2655f4e
--- /dev/null
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-dots-position-3.html b/LayoutTests/editing/spelling/spelling-dots-position-3.html
new file mode 100644 (file)
index 0000000..9fd7beb
--- /dev/null
@@ -0,0 +1,67 @@
+<!DOCTYPE html><!-- webkit-test-runner [ spellCheckingDots=true ] -->
+<html>
+<head>
+
+<style>
+.editing {
+    font-family: 'Times New Roman';
+    font-size: 36px;
+    color: transparent;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+function editingTest() {
+    typeCharacterCommand('w');
+    typeCharacterCommand('a');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('e');
+    typeCharacterCommand('r');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('d');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('a');
+    typeCharacterCommand(' ');
+    let ascentSetter = document.createElement("div");
+    ascentSetter.style.width = "1px";
+    ascentSetter.style.height = "150px";
+    ascentSetter.style.display = "inline-block";
+    let root = document.getElementById("root");
+    root.appendChild(ascentSetter);
+    let baseline = ascentSetter.offsetTop + ascentSetter.offsetHeight;
+    let left = root.offsetLeft;
+    let width = ascentSetter.offsetLeft - left;
+    let fontSize = Number.parseInt(window.getComputedStyle(root).getPropertyValue("font-size"));
+    let blocker = document.createElement("div");
+    blocker.style.background = "white";
+    blocker.style.position = "absolute";
+    blocker.style.left = `${left}px`;
+    blocker.style.top = `${baseline - 1 + fontSize * 0.11035}px`;
+    blocker.style.width = `${width}px`;
+    blocker.style.height = `${fontSize * 0.13247}px`;
+    document.body.appendChild(blocker);
+    root.blur();
+}
+
+</script>
+
+<title>Editing Test</title>
+</head>
+<body>
+<div contenteditable id="root" class="editing">
+<span id="test"></span>
+</div>
+
+<script>
+    if (window.internals)
+        internals.setContinuousSpellCheckingEnabled(true);
+    runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-dots-position-expected.html b/LayoutTests/editing/spelling/spelling-dots-position-expected.html
new file mode 100644 (file)
index 0000000..7daf53f
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.editing {
+    font-family: 'Times New Roman';
+    font-size: 36px;
+}
+</style>
+</head>
+<body>
+<div>There should be no misspelling dots visible below (because they're covered up by a div)</div>
+<div contenteditable="" id="root" class="editing">wafewerewdfea&nbsp;<span id="test"></span><div style="background-color: black; width: 1px; height: 150px; display: inline-block; background-position: initial initial; background-repeat: initial initial;"></div></div>
+<script>
+    runEditingTest();
+</script><div style="background-color: black; position: absolute; left: 8px; top: 179.9726px; width: 237px; height: 4.7689200000000005px; background-position: initial initial; background-repeat: initial initial;"></div>
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-dots-position.html b/LayoutTests/editing/spelling/spelling-dots-position.html
new file mode 100644 (file)
index 0000000..70c1dac
--- /dev/null
@@ -0,0 +1,68 @@
+<!DOCTYPE html><!-- webkit-test-runner [ spellCheckingDots=true ] -->
+<html>
+<head>
+
+<style>
+.editing {
+    font-family: 'Times New Roman';
+    font-size: 36px;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+function editingTest() {
+    typeCharacterCommand('w');
+    typeCharacterCommand('a');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('e');
+    typeCharacterCommand('r');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('d');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('a');
+    typeCharacterCommand(' ');
+    let ascentSetter = document.createElement("div");
+    ascentSetter.style.background = "black";
+    ascentSetter.style.width = "1px";
+    ascentSetter.style.height = "150px";
+    ascentSetter.style.display = "inline-block";
+    let root = document.getElementById("root");
+    root.appendChild(ascentSetter);
+    let baseline = ascentSetter.offsetTop + ascentSetter.offsetHeight;
+    let left = root.offsetLeft;
+    let width = ascentSetter.offsetLeft - left;
+    let fontSize = Number.parseInt(window.getComputedStyle(root).getPropertyValue("font-size"));
+    let blocker = document.createElement("div");
+    blocker.style.background = "black";
+    blocker.style.position = "absolute";
+    blocker.style.left = `${left}px`;
+    blocker.style.top = `${baseline + fontSize * 0.11035}px`;
+    blocker.style.width = `${width}px`;
+    blocker.style.height = `${fontSize * 0.13247}px`;
+    document.body.appendChild(blocker);
+    root.blur();
+}
+
+</script>
+
+<title>Editing Test</title>
+</head>
+<body>
+<div>There should be no misspelling dots visible below (because they're covered up by a div)</div>
+<div contenteditable id="root" class="editing">
+<span id="test"></span>
+</div>
+
+<script>
+    if (window.internals)
+        internals.setContinuousSpellCheckingEnabled(true);
+    runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-dots-repaint-expected.html b/LayoutTests/editing/spelling/spelling-dots-repaint-expected.html
new file mode 100644 (file)
index 0000000..f2848b7
--- /dev/null
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>Editing Test</title>
+</head>
+<body>
+This test passes if you don't see any dots below.
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-dots-repaint.html b/LayoutTests/editing/spelling/spelling-dots-repaint.html
new file mode 100644 (file)
index 0000000..ca1468b
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html><!-- webkit-test-runner [ spellCheckingDots=true ] -->
+<html>
+<head>
+
+<style>
+@font-face {
+    font-family: "WebFont";
+    src: url("resources/VerySmallDescentAhem.ttf") format("truetype");
+}
+.editing {
+    font: 36px "WebFont";
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+function editingTest() {
+    typeCharacterCommand('w');
+    typeCharacterCommand('a');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('e');
+    typeCharacterCommand('r');
+    typeCharacterCommand('e');
+    typeCharacterCommand('w');
+    typeCharacterCommand('d');
+    typeCharacterCommand('f');
+    typeCharacterCommand('e');
+    typeCharacterCommand('a');
+    typeCharacterCommand(' ');
+    window.setTimeout(function() {
+        let root = document.getElementById("root");
+        root.parentElement.removeChild(root);
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }, 20);
+}
+
+</script>
+
+<title>Editing Test</title>
+</head>
+<body>
+This test passes if you don't see any dots below.
+<div contenteditable id="root" class="editing">
+<span id="test"></span>
+</div>
+
+<script>
+    if (window.internals)
+        internals.setContinuousSpellCheckingEnabled(true);
+    if (window.testRunner)
+        testRunner.waitUntilDone();
+    runEditingTest();
+</script>
+
+</body>
+</html>
index c1f23aa..7ee1ed9 100644 (file)
@@ -1,3 +1,48 @@
+2018-11-05  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Spelling dots are drawn in the wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=190764
+
+        Reviewed by Dean Jackson.
+
+        - Dots should not be clipped.
+        - Dots should be horizontally centered.
+        - Dots should be drawn behind the text.
+        - Distance from the baseline to the top of the dot should be 11.035% of font size.
+        - Dot diameter should be 13.247% of the font size.
+        - Distance between the dots (right side of the left dot to left side of the right dot) should be 9.457% of the font size.
+        - The "font size" used in these calculations should be clamped so it's 10px <= font size <= 40px.
+
+        Tests: editing/spelling/spelling-dots-position-2.html
+               editing/spelling/spelling-dots-position-3.html
+               editing/spelling/spelling-dots-position.html
+               editing/spelling/spelling-dots-repaint.html
+
+        * platform/graphics/cocoa/GraphicsContextCocoa.mm:
+        (WebCore::colorForMarkerLineStyle): Align iOS and macOS implementations.
+        (WebCore::GraphicsContext::drawDotsForDocumentMarker): Place the dots correctly.
+        * rendering/InlineFlowBox.cpp:
+        (WebCore::InlineFlowBox::addToLine): The KnownToHaveNoOverflow flag should be cleared if the element has spelling dots,
+            because there is no guarantee the spelling dots will lie inside the layout rect of the element.
+        (WebCore::InlineFlowBox::addTextBoxVisualOverflow): Update the repaint rects to include splling dot positions.
+        * rendering/InlineFlowBox.h: Comments should explain why, not say what.
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paint): Draw the dots behind the text.
+        (WebCore::InlineTextBox::hasMarkers const): Convenience.
+        (WebCore::InlineTextBox::paintPlatformDocumentMarkers): Refactor bounds information into a helper function.
+        (WebCore::InlineTextBox::calculateUnionOfAllDocumentMarkerBounds const): Use for repaint rect calculation.
+        (WebCore::InlineTextBox::calculateDocumentMarkerBounds const): Place the dots correctly.
+        (WebCore::InlineTextBox::paintPlatformDocumentMarker): Call the helper method.
+        (WebCore::InlineTextBox::collectMarkedTextsForDocumentMarkers const):
+        (WebCore::InlineTextBox::collectMarkedTextsForDocumentMarkers): Deleted.
+        * rendering/InlineTextBox.h: Declare the helper methods.
+        * rendering/SimpleLineLayout.cpp: Simple line layout doesn't know how to paint spelling dots, so make the presence of
+            spelling dots opt us out of SLL.
+        (WebCore::SimpleLineLayout::canUseForWithReason):
+        * rendering/SimpleLineLayoutCoverage.cpp:
+        (WebCore::SimpleLineLayout::printReason):
+        * rendering/SimpleLineLayoutCoverage.h: Add a new opt-out reason.
+
 2018-11-05  Dean Jackson  <dino@apple.com>
 
         Attempted build fix.
index 48b5b94..bf5e8b1 100644 (file)
@@ -185,7 +185,6 @@ static inline void setPatternPhaseInUserSpace(CGContextRef context, CGPoint phas
 
 static CGColorRef colorForMarkerLineStyle(DocumentMarkerLineStyle::Mode style, bool useDarkMode)
 {
-#if PLATFORM(MAC)
     switch (style) {
     // Red
     case DocumentMarkerLineStyle::Mode::Spelling:
@@ -199,19 +198,6 @@ static CGColorRef colorForMarkerLineStyle(DocumentMarkerLineStyle::Mode style, b
     case DocumentMarkerLineStyle::Mode::Grammar:
         return cachedCGColor(useDarkMode ? Color { 50, 215, 75, 217 } : Color { 25, 175, 50, 191 });
     }
-#else
-    UNUSED_PARAM(useDarkMode);
-    switch (style) {
-    case DocumentMarkerLineStyle::Mode::Spelling:
-        return [getUIColorClass() systemRedColor].CGColor;
-    case DocumentMarkerLineStyle::Mode::DictationAlternatives:
-    case DocumentMarkerLineStyle::Mode::TextCheckingDictationPhraseWithAlternatives:
-    case DocumentMarkerLineStyle::Mode::AutocorrectionReplacement:
-        return [getUIColorClass() systemBlueColor].CGColor;
-    case DocumentMarkerLineStyle::Mode::Grammar:
-        return [getUIColorClass() systemGreenColor].CGColor;
-    }
-#endif
 }
 
 void GraphicsContext::drawDotsForDocumentMarker(const FloatRect& rect, DocumentMarkerLineStyle style)
@@ -219,28 +205,33 @@ void GraphicsContext::drawDotsForDocumentMarker(const FloatRect& rect, DocumentM
     if (paintingDisabled())
         return;
 
-    auto circleColor = colorForMarkerLineStyle(style.mode, style.shouldUseDarkAppearance);
-
-    auto lineThickness = rect.height();
-    auto patternGapWidth = lineThickness / 3;
-    auto patternWidth = lineThickness + patternGapWidth;
+    // We want to find the number of full dots, so we're solving the equations:
+    // dotDiameter = height
+    // dotDiameter / dotGap = 13.247 / 9.457
+    // numberOfGaps = numberOfDots - 1
+    // dotDiameter * numberOfDots + dotGap * numberOfGaps = width
 
-    FloatPoint offsetPoint = rect.location();
     auto width = rect.width();
-    float widthMod = fmodf(width, patternWidth);
-    if (patternWidth - widthMod > patternGapWidth) {
-        float gapIncludeWidth = 0;
-        if (width > patternWidth)
-            gapIncludeWidth = patternGapWidth;
-        offsetPoint.move(floor((widthMod + gapIncludeWidth) / 2), 0);
-        width -= widthMod;
-    }
+    auto dotDiameter = rect.height();
+    auto dotGap = dotDiameter * 9.457 / 13.247;
+    auto numberOfDots = (width + dotGap) / (dotDiameter + dotGap);
+    auto numberOfWholeDots = static_cast<unsigned>(numberOfDots);
+    auto numberOfWholeGaps = numberOfWholeDots - 1;
+
+    // Center the dots
+    auto offset = (width - (dotDiameter * numberOfWholeDots + dotGap * numberOfWholeGaps)) / 2;
+
+    auto circleColor = colorForMarkerLineStyle(style.mode, style.shouldUseDarkAppearance);
 
     CGContextRef platformContext = this->platformContext();
     CGContextStateSaver stateSaver { platformContext };
     CGContextSetFillColorWithColor(platformContext, circleColor);
-    for (int x = 0; x < width; x += patternWidth)
-        CGContextAddEllipseInRect(platformContext, CGRectMake(offsetPoint.x() + x, offsetPoint.y(), lineThickness, lineThickness));
+    for (unsigned i = 0; i < numberOfWholeDots; ++i) {
+        auto location = rect.location();
+        location.move(offset + i * (dotDiameter + dotGap), 0);
+        auto size = FloatSize(dotDiameter, dotDiameter);
+        CGContextAddEllipseInRect(platformContext, FloatRect(location, size));
+    }
     CGContextSetCompositeOperation(platformContext, kCGCompositeSover);
     CGContextFillPath(platformContext);
 }
index 2876b17..09501f7 100644 (file)
@@ -158,7 +158,12 @@ void InlineFlowBox::addToLine(InlineBox* child)
         const RenderStyle& childStyle = child->lineStyle();
         if (child->behavesLikeText()) {
             const RenderStyle* childStyle = &child->lineStyle();
-            if (childStyle->letterSpacing() < 0 || childStyle->textShadow() || childStyle->textEmphasisMark() != TextEmphasisMark::None || childStyle->hasPositiveStrokeWidth())
+            bool hasMarkers = false;
+            if (is<InlineTextBox>(child)) {
+                const auto* textBox = downcast<InlineTextBox>(child);
+                hasMarkers = textBox->hasMarkers();
+            }
+            if (childStyle->letterSpacing() < 0 || childStyle->textShadow() || childStyle->textEmphasisMark() != TextEmphasisMark::None || childStyle->hasPositiveStrokeWidth() || hasMarkers)
                 child->clearKnownToHaveNoOverflow();
         } else if (child->renderer().isReplaced()) {
             const RenderBox& box = downcast<RenderBox>(child->renderer());
@@ -928,6 +933,10 @@ inline void InlineFlowBox::addTextBoxVisualOverflow(InlineTextBox& textBox, Glyp
     
     logicalVisualOverflow = LayoutRect(logicalLeftVisualOverflow, logicalTopVisualOverflow,
                                        logicalRightVisualOverflow - logicalLeftVisualOverflow, logicalBottomVisualOverflow - logicalTopVisualOverflow);
+
+    auto documentMarkerBounds = textBox.calculateUnionOfAllDocumentMarkerBounds();
+    documentMarkerBounds.move(textBox.logicalLeft(), textBox.logicalTop());
+    logicalVisualOverflow = unionRect(logicalVisualOverflow, LayoutRect(documentMarkerBounds));
                                     
     textBox.setLogicalOverflowRect(logicalVisualOverflow);
 }
index 27c6229..0991aa7 100644 (file)
@@ -292,8 +292,7 @@ public:
     }
 
     void computeReplacedAndTextLineTopAndBottom(LayoutUnit& lineTop, LayoutUnit& lineBottom) const;
-    
-    // Used to calculate the underline offset for TextUnderlinePosition::Under.
+
     void maxLogicalBottomForTextDecorationLine(float& maxLogicalBottom, const RenderElement* decorationRenderer, OptionSet<TextDecoration>) const;
     void minLogicalTopForTextDecorationLine(float& minLogicalTop, const RenderElement* decorationRenderer, OptionSet<TextDecoration>) const;
 
index 7cbc078..9e71a71 100644 (file)
@@ -543,6 +543,9 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,
     if (paintInfo.phase == PaintPhase::Foreground)
         renderer().page().addRelevantRepaintedObject(&renderer(), IntRect(boxOrigin.x(), boxOrigin.y(), logicalWidth(), logicalHeight()));
 
+    if (paintInfo.phase == PaintPhase::Foreground)
+        paintPlatformDocumentMarkers(context, boxOrigin);
+
     // 2. Now paint the foreground, including text and decorations like underline/overline (in quirks mode only).
     bool shouldPaintSelectionForeground = haveSelection && !useCustomUnderlines;
     Vector<MarkedText> markedTexts;
@@ -620,12 +623,8 @@ void InlineTextBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset,
     }
 
     // 3. Paint fancy decorations, including composition underlines and platform-specific underlines for spelling errors, grammar errors, et cetera.
-    if (paintInfo.phase == PaintPhase::Foreground) {
-        paintPlatformDocumentMarkers(context, boxOrigin);
-
-        if (useCustomUnderlines)
-            paintCompositionUnderlines(paintInfo, boxOrigin);
-    }
+    if (paintInfo.phase == PaintPhase::Foreground && useCustomUnderlines)
+        paintCompositionUnderlines(paintInfo, boxOrigin);
     
     if (shouldRotate)
         context.concatCTM(rotation(boxRect, Counterclockwise));
@@ -665,12 +664,46 @@ std::pair<unsigned, unsigned> InlineTextBox::selectionStartEnd() const
     return { clampedOffset(start), clampedOffset(end) };
 }
 
+bool InlineTextBox::hasMarkers() const
+{
+    return collectMarkedTextsForDocumentMarkers(TextPaintPhase::Decoration).size();
+}
+
 void InlineTextBox::paintPlatformDocumentMarkers(GraphicsContext& context, const FloatPoint& boxOrigin)
 {
+    // This must match calculateUnionOfAllDocumentMarkerBounds().
     for (auto& markedText : subdivide(collectMarkedTextsForDocumentMarkers(TextPaintPhase::Decoration), OverlapStrategy::Frontmost))
         paintPlatformDocumentMarker(context, boxOrigin, markedText);
 }
 
+FloatRect InlineTextBox::calculateUnionOfAllDocumentMarkerBounds() const
+{
+    // This must match paintPlatformDocumentMarkers().
+    FloatRect result;
+    for (auto& markedText : subdivide(collectMarkedTextsForDocumentMarkers(TextPaintPhase::Decoration), OverlapStrategy::Frontmost))
+        result = unionRect(result, calculateDocumentMarkerBounds(markedText));
+    return result;
+}
+
+FloatRect InlineTextBox::calculateDocumentMarkerBounds(const MarkedText& markedText) const
+{
+    auto& font = lineFont();
+    auto ascent = font.fontMetrics().ascent();
+    auto fontSize = std::min(std::max(font.size(), 10.0f), 40.0f);
+    auto y = ascent + 0.11035 * fontSize;
+    auto height = 0.13247 * fontSize;
+
+    // Avoid measuring the text when the entire line box is selected as an optimization.
+    if (markedText.startOffset || markedText.endOffset != clampedOffset(end() + 1)) {
+        TextRun run = createTextRun();
+        LayoutRect selectionRect = LayoutRect(0, y, 0, height);
+        lineFont().adjustSelectionRectForText(run, selectionRect, markedText.startOffset, markedText.endOffset);
+        return selectionRect;
+    }
+
+    return FloatRect(0, y, logicalWidth(), height);
+}
+
 void InlineTextBox::paintPlatformDocumentMarker(GraphicsContext& context, const FloatPoint& boxOrigin, const MarkedText& markedText)
 {
     // Never print spelling/grammar markers (5327887)
@@ -680,27 +713,11 @@ void InlineTextBox::paintPlatformDocumentMarker(GraphicsContext& context, const
     if (m_truncation == cFullTruncation)
         return;
 
-    float start = 0; // start of line to draw, relative to tx
-    float width = logicalWidth(); // how much line to draw
+    auto bounds = calculateDocumentMarkerBounds(markedText);
 
-    // Avoid measuring the text when the entire line box is selected as an optimization.
-    if (markedText.startOffset || markedText.endOffset != clampedOffset(end() + 1)) {
-        // Calculate start & width
-        int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
-        int selHeight = selectionHeight();
-        FloatPoint startPoint(boxOrigin.x(), boxOrigin.y() - deltaY);
-        TextRun run = createTextRun();
-
-        LayoutRect selectionRect = LayoutRect(startPoint, FloatSize(0, selHeight));
-        lineFont().adjustSelectionRectForText(run, selectionRect, markedText.startOffset, markedText.endOffset);
-        IntRect markerRect = enclosingIntRect(selectionRect);
-        start = markerRect.x() - startPoint.x();
-        width = markerRect.width();
-    }
-
-    auto lineStyleForMarkedTextType = [&] (MarkedText::Type type) -> DocumentMarkerLineStyle {
+    auto lineStyleForMarkedTextType = [&]() -> DocumentMarkerLineStyle {
         bool shouldUseDarkAppearance = renderer().document().useDarkAppearance();
-        switch (type) {
+        switch (markedText.type) {
         case MarkedText::SpellingError:
             return { DocumentMarkerLineStyle::Mode::Spelling, shouldUseDarkAppearance };
         case MarkedText::GrammarError:
@@ -720,25 +737,8 @@ void InlineTextBox::paintPlatformDocumentMarker(GraphicsContext& context, const
         }
     };
 
-    // IMPORTANT: The misspelling underline is not considered when calculating the text bounds, so we have to
-    // make sure to fit within those bounds.  This means the top pixel(s) of the underline will overlap the
-    // bottom pixel(s) of the glyphs in smaller font sizes.  The alternatives are to increase the line spacing (bad!!)
-    // or decrease the underline thickness.  The overlap is actually the most useful, and matches what AppKit does.
-    // So, we generally place the underline at the bottom of the text, but in larger fonts that's not so good so
-    // we pin to two pixels under the baseline.
-    int lineThickness = 3;
-    int baseline = lineStyle().fontMetrics().ascent();
-    int descent = logicalHeight() - baseline;
-    int underlineOffset;
-    if (descent <= (2 + lineThickness)) {
-        // Place the underline at the very bottom of the text in small/medium fonts.
-        underlineOffset = logicalHeight() - lineThickness;
-    } else {
-        // In larger fonts, though, place the underline up near the baseline to prevent a big gap.
-        underlineOffset = baseline + 2;
-    }
-
-    context.drawDotsForDocumentMarker(FloatRect(boxOrigin.x() + start, boxOrigin.y() + underlineOffset, width, lineThickness), lineStyleForMarkedTextType(markedText.type));
+    bounds.moveBy(boxOrigin);
+    context.drawDotsForDocumentMarker(bounds, lineStyleForMarkedTextType());
 }
 
 auto InlineTextBox::computeStyleForUnmarkedMarkedText(const PaintInfo& paintInfo) const -> MarkedTextStyle
@@ -855,7 +855,7 @@ Vector<MarkedText> InlineTextBox::collectMarkedTextsForDraggedContent()
     return result;
 }
 
-Vector<MarkedText> InlineTextBox::collectMarkedTextsForDocumentMarkers(TextPaintPhase phase)
+Vector<MarkedText> InlineTextBox::collectMarkedTextsForDocumentMarkers(TextPaintPhase phase) const
 {
     ASSERT_ARG(phase, phase == TextPaintPhase::Background || phase == TextPaintPhase::Foreground || phase == TextPaintPhase::Decoration);
 
index 37ee5e1..ae77a66 100644 (file)
@@ -157,6 +157,10 @@ public:
     virtual int offsetForPosition(float x, bool includePartialGlyphs = true) const;
     virtual float positionForOffset(unsigned offset) const;
 
+    bool hasMarkers() const;
+    FloatRect calculateUnionOfAllDocumentMarkerBounds() const;
+    FloatRect calculateDocumentMarkerBounds(const MarkedText&) const;
+
 private:
     struct MarkedTextStyle;
     struct StyledMarkedText;
@@ -164,7 +168,7 @@ private:
     enum class TextPaintPhase { Background, Foreground, Decoration };
 
     Vector<MarkedText> collectMarkedTextsForDraggedContent();
-    Vector<MarkedText> collectMarkedTextsForDocumentMarkers(TextPaintPhase);
+    Vector<MarkedText> collectMarkedTextsForDocumentMarkers(TextPaintPhase) const;
 
     MarkedTextStyle computeStyleForUnmarkedMarkedText(const PaintInfo&) const;
     StyledMarkedText resolveStyleForMarkedText(const MarkedText&, const MarkedTextStyle& baseStyle, const PaintInfo&);
index e0c7bea..e3fea3f 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "SimpleLineLayout.h"
 
+#include "DocumentMarkerController.h"
 #include "FontCache.h"
 #include "Frame.h"
 #include "GraphicsContext.h"
@@ -313,6 +314,9 @@ AvoidanceReasonFlags canUseForWithReason(const RenderBlockFlow& flow, IncludeRea
         if (child->selectionState() != RenderObject::SelectionNone)
             SET_REASON_AND_RETURN_IF_NEEDED(FlowChildIsSelected, reasons, includeReasons);
         if (is<RenderText>(*child)) {
+            const auto& renderText = downcast<RenderText>(*child);
+            if (!renderText.document().markers().markersFor(renderText.textNode()).isEmpty())
+                SET_REASON_AND_RETURN_IF_NEEDED(FlowIncludesDocumentMarkers, reasons, includeReasons);
             child = child->nextSibling();
             continue;
         }
index b29235b..f6ba927 100644 (file)
@@ -193,7 +193,10 @@ static void printReason(AvoidanceReason reason, TextStream& stream)
         stream << "column with vertical-align != baseline";
         break;
     case MultiColumnFlowIsFloating:
-        stream << "column with floating objecgts";
+        stream << "column with floating objects";
+        break;
+    case FlowIncludesDocumentMarkers:
+        stream << "text includes document markers";
         break;
     case FlowTextIsEmpty:
     case FlowHasNoChild:
index cda44dd..5581470 100644 (file)
@@ -90,7 +90,8 @@ enum AvoidanceReason_ : uint64_t {
     MultiColumnFlowHasColumnSpanner       = 1LLU  << 52,
     MultiColumnFlowVerticalAlign          = 1LLU  << 53,
     MultiColumnFlowIsFloating             = 1LLU  << 54,
-    EndOfReasons                          = 1LLU  << 55
+    FlowIncludesDocumentMarkers           = 1LLU  << 55,
+    EndOfReasons                          = 1LLU  << 56
 };
 const unsigned NoReason = 0;
 
index abe33f4..694d87b 100644 (file)
@@ -1,3 +1,29 @@
+2018-11-05  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Spelling dots are drawn in the wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=190764
+
+        Reviewed by Dean Jackson.
+
+        Previously, it was impossible for WebKitTestRunner to draw spelling dots. This patch adds support for a header
+        at the top of test files, of the form <!-- webkit-test-runner [ spellCheckingDots=true ] --> which will cause
+        dots to be drawn.
+
+        * WebKitTestRunner/TestController.cpp:
+        (WTR::TestController::resetStateToConsistentValues):
+        (WTR::updateTestOptionsFromTestHeader):
+        (WTR::TestController::platformResetStateToConsistentValues):
+        * WebKitTestRunner/TestController.h:
+        * WebKitTestRunner/TestOptions.h:
+        (WTR::TestOptions::hasSameInitializationOptions const):
+        * WebKitTestRunner/cocoa/TestControllerCocoa.mm:
+        (WTR::TestController::cocoaResetStateToConsistentValues):
+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
+        * WebKitTestRunner/ios/TestControllerIOS.mm:
+        (WTR::TestController::platformResetStateToConsistentValues):
+        * WebKitTestRunner/mac/TestControllerMac.mm:
+        (WTR::TestController::platformResetStateToConsistentValues):
+
 2018-11-05  Chris Dumez  <cdumez@apple.com>
 
         Use same limit for page cache and suspended pages
index 817f0f4..d55a9fc 100644 (file)
@@ -926,7 +926,7 @@ bool TestController::resetStateToConsistentValues(const TestOptions& options, Re
 
     setHidden(false);
 
-    platformResetStateToConsistentValues();
+    platformResetStateToConsistentValues(options);
 
     m_shouldDecideNavigationPolicyAfterDelay = false;
     m_shouldDecideResponsePolicyAfterDelay = false;
@@ -1237,6 +1237,8 @@ static void updateTestOptionsFromTestHeader(TestOptions& testOptions, const std:
             testOptions.runSingly = parseBooleanTestHeaderValue(value);
         else if (key == "shouldIgnoreMetaViewport")
             testOptions.shouldIgnoreMetaViewport = parseBooleanTestHeaderValue(value);
+        else if (key == "spellCheckingDots")
+            testOptions.shouldShowSpellCheckingDots = parseBooleanTestHeaderValue(value);
         pairStart = pairEnd + 1;
     }
 }
@@ -2649,7 +2651,7 @@ WKContextRef TestController::platformAdjustContext(WKContextRef context, WKConte
     return context;
 }
 
-void TestController::platformResetStateToConsistentValues()
+void TestController::platformResetStateToConsistentValues(const TestOptions&)
 {
 }
 
index 93e8886..a0c906b 100644 (file)
@@ -283,10 +283,10 @@ private:
     void platformCreateWebView(WKPageConfigurationRef, const TestOptions&);
     static PlatformWebView* platformCreateOtherPage(PlatformWebView* parentView, WKPageConfigurationRef, const TestOptions&);
     void platformResetPreferencesToConsistentValues();
-    void platformResetStateToConsistentValues();
+    void platformResetStateToConsistentValues(const TestOptions&);
 #if PLATFORM(COCOA)
     void cocoaPlatformInitialize();
-    void cocoaResetStateToConsistentValues();
+    void cocoaResetStateToConsistentValues(const TestOptions&);
 #endif
     void platformConfigureViewForTest(const TestInvocation&);
     void platformWillRunTest(const TestInvocation&);
index acf95b4..917c12a 100644 (file)
@@ -63,6 +63,7 @@ struct TestOptions {
     bool runSingly { false };
     bool checkForWorldLeaks { false };
     bool shouldIgnoreMetaViewport { false };
+    bool shouldShowSpellCheckingDots { false };
 
     float deviceScaleFactor { 1 };
     Vector<String> overrideLanguages;
@@ -103,7 +104,8 @@ struct TestOptions {
             || jscOptions != options.jscOptions
             || runSingly != options.runSingly
             || checkForWorldLeaks != options.checkForWorldLeaks
-            || shouldIgnoreMetaViewport != options.shouldIgnoreMetaViewport)
+            || shouldIgnoreMetaViewport != options.shouldIgnoreMetaViewport
+            || shouldShowSpellCheckingDots != options.shouldShowSpellCheckingDots)
             return false;
 
         if (experimentalFeatures != options.experimentalFeatures)
index 959c8f3..bb5e71d 100644 (file)
@@ -203,7 +203,7 @@ void TestController::platformRunUntil(bool& done, WTF::Seconds timeout)
         [[NSRunLoop currentRunLoop] runMode:NSDefaultRunLoopMode beforeDate:endDate];
 }
 
-void TestController::cocoaResetStateToConsistentValues()
+void TestController::cocoaResetStateToConsistentValues(const TestOptions& options)
 {
 #if WK_API_ENABLED
     __block bool doneRemoving = false;
@@ -213,10 +213,15 @@ void TestController::cocoaResetStateToConsistentValues()
     platformRunUntil(doneRemoving, noTimeout);
     [[_WKUserContentExtensionStore defaultStore] _removeAllContentExtensions];
 
+
     if (auto* webView = mainWebView()) {
         TestRunnerWKWebView *platformView = webView->platformView();
         [platformView.configuration.userContentController _removeAllUserContentFilters];
         platformView._viewScale = 1;
+
+        // Toggle on before the test, and toggle off after the test.
+        if (options.shouldShowSpellCheckingDots)
+            [platformView toggleContinuousSpellChecking:nil];
     }
 #endif
 }
index fcdeb51..5bc2cb7 100644 (file)
 
 #if WK_API_ENABLED
 
+@interface WKWebView(SpellChecking)
+- (IBAction)toggleContinuousSpellChecking:(id)sender;
+@end
+
 @interface TestRunnerWKWebView : WKWebView
 
 #if PLATFORM(IOS_FAMILY)
index ab4af11..7b5146a 100644 (file)
@@ -101,9 +101,9 @@ void TestController::platformResetPreferencesToConsistentValues()
     WKPreferencesSetTextAutosizingEnabled(preferences, false);
 }
 
-void TestController::platformResetStateToConsistentValues()
+void TestController::platformResetStateToConsistentValues(const TestOptions& options)
 {
-    cocoaResetStateToConsistentValues();
+    cocoaResetStateToConsistentValues(options);
 
     [[UIDevice currentDevice] setOrientation:UIDeviceOrientationPortrait animated:NO];
     
index f9dd39d..053633b 100644 (file)
@@ -102,9 +102,9 @@ void TestController::platformResetPreferencesToConsistentValues()
 {
 }
 
-void TestController::platformResetStateToConsistentValues()
+void TestController::platformResetStateToConsistentValues(const TestOptions& options)
 {
-    cocoaResetStateToConsistentValues();
+    cocoaResetStateToConsistentValues(options);
 
     while ([NSApp nextEventMatchingMask:NSEventMaskGesture | NSEventMaskScrollWheel untilDate:nil inMode:NSDefaultRunLoopMode dequeue:YES]) {
         // Clear out (and ignore) any pending gesture and scroll wheel events.