Spelling error annotation should encompass hyphen in misspelled word that wraps acros...
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Oct 2017 00:18:04 +0000 (00:18 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Oct 2017 00:18:04 +0000 (00:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177980
<rdar://problem/34847454>

Reviewed by Simon Fraser.

Source/WebCore:

On macOS the spelling and grammar annotations for a word or word phrase encompass
hyphenations added because the word or word phrase wraps across more than one line.
The effect tends to be more aesthetically pleasing and consistent with how these
annotations would be pointed out by a person in conversation: by identify the word
or phrase that has a spelling or grammar issue regardless of whether that word or
phrase is broken into halves due to line wrapping. The same argument applies to
other annotations on macOS, including text matches. Therefore, we should always
include any hyphens encompassed by a marker that were added due to line wrapping
when painting the marker.

Test: editing/spelling/spelling-marker-includes-hyphen.html

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paintDocumentMarker): Compute the text run including any
added hyphens. If a hyphen was added then the inline text box represents that text
up to the hyphen. Adjust the end position of the marker to be the length of the text
run if its greater than or equal to the length of the text box.

LayoutTests:

Add a test to ensure that a spelling error decoration encompasses the hyphen for a misspelled
word that is hyphenated because it is broken across more than one line.

* editing/spelling/spelling-marker-includes-hyphen-expected.html: Added.
* editing/spelling/spelling-marker-includes-hyphen.html: Added.
* platform/ios/TestExpectations: Mark the test as WontFix as spelling and
grammar markers are not support on iOS.
* platform/mac-wk2/TestExpectations: Mark the test as a failure due to <https://bugs.webkit.org/show_bug.cgi?id=105616>.

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

LayoutTests/ChangeLog
LayoutTests/editing/spelling/spelling-marker-includes-hyphen-expected.html [new file with mode: 0644]
LayoutTests/editing/spelling/spelling-marker-includes-hyphen.html [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/rendering/InlineTextBox.cpp

index ff03d50..b5dda42 100644 (file)
@@ -1,3 +1,20 @@
+2017-10-06  Daniel Bates  <dabates@apple.com>
+
+        Spelling error annotation should encompass hyphen in misspelled word that wraps across multiple lines
+        https://bugs.webkit.org/show_bug.cgi?id=177980
+        <rdar://problem/34847454>
+
+        Reviewed by Simon Fraser.
+
+        Add a test to ensure that a spelling error decoration encompasses the hyphen for a misspelled
+        word that is hyphenated because it is broken across more than one line.
+
+        * editing/spelling/spelling-marker-includes-hyphen-expected.html: Added.
+        * editing/spelling/spelling-marker-includes-hyphen.html: Added.
+        * platform/ios/TestExpectations: Mark the test as WontFix as spelling and
+        grammar markers are not support on iOS.
+        * platform/mac-wk2/TestExpectations: Mark the test as a failure due to <https://bugs.webkit.org/show_bug.cgi?id=105616>.
+
 2017-10-06  Matt Baker  <mattbaker@apple.com>
 
         Web Inspector: Add Canvas tab and CanvasOverviewContentView
diff --git a/LayoutTests/editing/spelling/spelling-marker-includes-hyphen-expected.html b/LayoutTests/editing/spelling/spelling-marker-includes-hyphen-expected.html
new file mode 100644 (file)
index 0000000..858b132
--- /dev/null
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: Ahem;
+    src: url("../../resources/Ahem.ttf");
+}
+#test {
+    font-family: Ahem;
+    width: 100px;
+}
+</style>
+</head>
+<body>
+<div id="test" contenteditable="true"></div>
+<script>
+var testElement = document.getElementById("test");
+testElement.focus();
+document.execCommand("InsertText", false, "misllHlll"); // The H is a placeholder for the hyphen (had it been automatically inserted).
+document.execCommand("InsertText", false, " "); // Trigger spell checker
+testElement.blur();
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/spelling/spelling-marker-includes-hyphen.html b/LayoutTests/editing/spelling/spelling-marker-includes-hyphen.html
new file mode 100644 (file)
index 0000000..6424dac
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@font-face {
+    font-family: Ahem;
+    src: url("../../resources/Ahem.ttf");
+}
+#test {
+    font-family: Ahem;
+    -webkit-hyphens: auto;
+    hypens: auto;
+    width: 100px;
+}
+</style>
+</head>
+<body>
+<div id="test" contenteditable="true"></div>
+<script>
+var testElement = document.getElementById("test");
+testElement.focus();
+document.execCommand("InsertText", false, "mislllll");
+document.execCommand("InsertText", false, " "); // Trigger spell checker
+testElement.blur();
+</script>
+</body>
+</html>
index 03fe6d7..3c1427a 100644 (file)
@@ -91,6 +91,9 @@ mhtml
 # ENABLE(MEDIA_CAPTURE) is not enabled
 fast/forms/file/file-input-capture.html
 
+# Spelling and grammar markers are not supported
+webkit.org/b/105616 editing/spelling/spelling-marker-includes-hyphen.html [ WontFix ]
+
 # Plugins are not supported on iOS
 plugins
 compositing/plugins
index eb2e3d1..7653455 100644 (file)
@@ -203,6 +203,7 @@ webkit.org/b/105616 editing/mac/spelling/move-cursor-to-beginning-of-autocorrect
 webkit.org/b/105616 editing/mac/spelling/removing-underline-after-accepting-autocorrection-using-punctuation.html [ Failure ]
 webkit.org/b/105616 editing/mac/spelling/accept-candidate-allows-autocorrect-on-next-word.html [ Failure ]
 webkit.org/b/105616 editing/mac/spelling/accept-unseen-candidate-records-acceptance.html [ Failure ]
+webkit.org/b/105616 editing/spelling/spelling-marker-includes-hyphen.html [ ImageOnlyFailure ]
 
 # [WK2] [Mac] Support drag in mouse events for WebKit2 EventSender
 # <https://bugs.webkit.org/show_bug.cgi?id=68552>
index 56bb093..74eded0 100644 (file)
@@ -1,3 +1,29 @@
+2017-10-06  Daniel Bates  <dabates@apple.com>
+
+        Spelling error annotation should encompass hyphen in misspelled word that wraps across multiple lines
+        https://bugs.webkit.org/show_bug.cgi?id=177980
+        <rdar://problem/34847454>
+
+        Reviewed by Simon Fraser.
+
+        On macOS the spelling and grammar annotations for a word or word phrase encompass
+        hyphenations added because the word or word phrase wraps across more than one line.
+        The effect tends to be more aesthetically pleasing and consistent with how these
+        annotations would be pointed out by a person in conversation: by identify the word
+        or phrase that has a spelling or grammar issue regardless of whether that word or
+        phrase is broken into halves due to line wrapping. The same argument applies to
+        other annotations on macOS, including text matches. Therefore, we should always
+        include any hyphens encompassed by a marker that were added due to line wrapping
+        when painting the marker.
+
+        Test: editing/spelling/spelling-marker-includes-hyphen.html
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paintDocumentMarker): Compute the text run including any
+        added hyphens. If a hyphen was added then the inline text box represents that text
+        up to the hyphen. Adjust the end position of the marker to be the length of the text
+        run if its greater than or equal to the length of the text box.
+
 2017-10-06  Zalan Bujtas  <zalan@apple.com>
 
         RenderTable should not hold a collection of raw pointers to RenderTableCol
index 863d0a2..d800b0a 100644 (file)
@@ -812,22 +812,21 @@ void InlineTextBox::paintDocumentMarker(GraphicsContext& context, const FloatPoi
     if (!markerSpansWholeBox) {
         unsigned startPosition = clampedOffset(subrange.startOffset);
         unsigned endPosition = clampedOffset(subrange.endOffset);
-        
+
         if (m_truncation != cNoTruncation)
             endPosition = std::min(endPosition, static_cast<unsigned>(m_truncation));
 
         // Calculate start & width
-        // FIXME: Adjust text run for combined text and hyphenation.
+        // FIXME: Adjust text run for combined text.
         bool ignoreCombinedText = true;
-        bool ignoreHyphen = true;
         int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
         int selHeight = selectionHeight();
         FloatPoint startPoint(boxOrigin.x(), boxOrigin.y() - deltaY);
-        auto text = this->text(ignoreCombinedText, ignoreHyphen);
+        auto text = this->text(ignoreCombinedText);
         TextRun run = createTextRun(text);
 
         LayoutRect selectionRect = LayoutRect(startPoint, FloatSize(0, selHeight));
-        lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition);
+        lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition >= len() ? run.length() : endPosition);
         IntRect markerRect = enclosingIntRect(selectionRect);
         start = markerRect.x() - startPoint.x();
         width = markerRect.width();