Title: [254912] releases/WebKitGTK/webkit-2.26
- Revision
- 254912
- Author
- [email protected]
- Date
- 2020-01-22 02:42:35 -0800 (Wed, 22 Jan 2020)
Log Message
Merge r254114 - ComplexTextController::offsetForPosition returns a wrong offset for a glyph boundary in a RTL text
https://bugs.webkit.org/show_bug.cgi?id=205486
Reviewed by Ross Kirsling.
Source/WebCore:
ComplexTextController::offsetForPosition had the following code:
> unsigned hitIndex = hitGlyphStart + (hitGlyphEnd - hitGlyphStart) * (m_run.ltr() ? x / adjustedAdvance : 1 - x / adjustedAdvance);
If m_run.ltr() was false and x == 0, hitIndex would become hitGlyphEnd.
This is not expected. It expects hitIndex < hitGlyphEnd if hitGlyphStart ≠ hitGlyphEnd.
Let hitIndex be hitGlyphStart-1 in the such condition.
Above change makes fast/text/ellipsis-text-rtl.html starting to
fail because offsetForPosition returns the character offset of the
next glyph if the argument 'h' is in a glyph boundary. In RTL
text, offsetForPosition should return a character offset of the
previous glyph in case of a glyph boundary. Use '<=' instead of '<'
for RTL text in order to select previous glyphs for glyph
boundaries.
Test: fast/dom/Document/CaretRangeFromPoint/rtl.html
* platform/graphics/ComplexTextController.cpp:
(WebCore::ComplexTextController::offsetForPosition): Compute correct hitGlyphEnd for RTL.
LayoutTests:
* fast/dom/Document/CaretRangeFromPoint/rtl-expected.txt: Added.
* fast/dom/Document/CaretRangeFromPoint/rtl.html: Added.
* platform/gtk/TestExpectations: Unmarked imported/blink/editing/selection/offset-from-point-complex-scripts.html.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.26/LayoutTests/ChangeLog (254911 => 254912)
--- releases/WebKitGTK/webkit-2.26/LayoutTests/ChangeLog 2020-01-22 10:42:29 UTC (rev 254911)
+++ releases/WebKitGTK/webkit-2.26/LayoutTests/ChangeLog 2020-01-22 10:42:35 UTC (rev 254912)
@@ -1,3 +1,14 @@
+2020-01-06 Fujii Hironori <[email protected]>
+
+ ComplexTextController::offsetForPosition returns a wrong offset for a glyph boundary in a RTL text
+ https://bugs.webkit.org/show_bug.cgi?id=205486
+
+ Reviewed by Ross Kirsling.
+
+ * fast/dom/Document/CaretRangeFromPoint/rtl-expected.txt: Added.
+ * fast/dom/Document/CaretRangeFromPoint/rtl.html: Added.
+ * platform/gtk/TestExpectations: Unmarked imported/blink/editing/selection/offset-from-point-complex-scripts.html.
+
2020-01-08 Fujii Hironori <[email protected]>
[HarfBuzz][GTK] fast/text/complex-first-glyph-with-initial-advance.html is failing
Added: releases/WebKitGTK/webkit-2.26/LayoutTests/fast/dom/Document/CaretRangeFromPoint/rtl-expected.txt (0 => 254912)
--- releases/WebKitGTK/webkit-2.26/LayoutTests/fast/dom/Document/CaretRangeFromPoint/rtl-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.26/LayoutTests/fast/dom/Document/CaretRangeFromPoint/rtl-expected.txt 2020-01-22 10:42:35 UTC (rev 254912)
@@ -0,0 +1 @@
+6 5 4 3 2 1 0
Added: releases/WebKitGTK/webkit-2.26/LayoutTests/fast/dom/Document/CaretRangeFromPoint/rtl.html (0 => 254912)
--- releases/WebKitGTK/webkit-2.26/LayoutTests/fast/dom/Document/CaretRangeFromPoint/rtl.html (rev 0)
+++ releases/WebKitGTK/webkit-2.26/LayoutTests/fast/dom/Document/CaretRangeFromPoint/rtl.html 2020-01-22 10:42:35 UTC (rev 254912)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<head>
+ <meta charset="utf-8">
+ <style>
+ p {
+ font-family: "Ahem";
+ direction: rtl;
+ unicode-bidi: bidi-override;
+ }
+ </style>
+</head>
+<div id="container">
+ <p>ABCDEF</p>
+</div>
+<div id=log></div>
+<script>
+testOffsetFromPoint(container.firstElementChild);
+
+function testOffsetFromPoint(element) {
+ var y = element.offsetTop + element.offsetHeight / 2;
+ var xmin = element.offsetLeft;
+ var xmax = xmin + element.offsetWidth;
+ var lastCharacterOffset = null;
+ var results = [];
+ for (var x = xmin - 1; x <= xmax + 1; ++x) {
+ var result = document.caretRangeFromPoint(x, y);
+ var characterOffset = result ? result.startOffset : null;
+ if (characterOffset === lastCharacterOffset)
+ continue;
+ results.push(characterOffset);
+ lastCharacterOffset = characterOffset;
+ }
+ var div = document.createElement("div");
+ div.innerText = results.join(" ");
+ log.appendChild(div);
+}
+
+if (window.testRunner) {
+ container.style.display = "none";
+ testRunner.dumpAsText();
+}
+</script>
Modified: releases/WebKitGTK/webkit-2.26/LayoutTests/platform/gtk/TestExpectations (254911 => 254912)
--- releases/WebKitGTK/webkit-2.26/LayoutTests/platform/gtk/TestExpectations 2020-01-22 10:42:29 UTC (rev 254911)
+++ releases/WebKitGTK/webkit-2.26/LayoutTests/platform/gtk/TestExpectations 2020-01-22 10:42:35 UTC (rev 254912)
@@ -3372,7 +3372,6 @@
webkit.org/b/177936 fast/text/all-small-caps-whitespace.html [ ImageOnlyFailure ]
webkit.org/b/177936 fast/text/regional-indicator-symobls.html [ Failure ]
webkit.org/b/177936 fast/text/selection-in-initial-advance-region.html [ Failure ]
-webkit.org/b/177936 imported/blink/editing/selection/offset-from-point-complex-scripts.html [ Failure ]
webkit.org/b/177937 fast/text/soft-hyphen-min-preferred-width.html [ ImageOnlyFailure ]
webkit.org/b/177937 fast/text/unknown-char-notdef.html [ ImageOnlyFailure ]
Modified: releases/WebKitGTK/webkit-2.26/Source/WebCore/ChangeLog (254911 => 254912)
--- releases/WebKitGTK/webkit-2.26/Source/WebCore/ChangeLog 2020-01-22 10:42:29 UTC (rev 254911)
+++ releases/WebKitGTK/webkit-2.26/Source/WebCore/ChangeLog 2020-01-22 10:42:35 UTC (rev 254912)
@@ -1,3 +1,31 @@
+2020-01-06 Fujii Hironori <[email protected]>
+
+ ComplexTextController::offsetForPosition returns a wrong offset for a glyph boundary in a RTL text
+ https://bugs.webkit.org/show_bug.cgi?id=205486
+
+ Reviewed by Ross Kirsling.
+
+ ComplexTextController::offsetForPosition had the following code:
+
+ > unsigned hitIndex = hitGlyphStart + (hitGlyphEnd - hitGlyphStart) * (m_run.ltr() ? x / adjustedAdvance : 1 - x / adjustedAdvance);
+
+ If m_run.ltr() was false and x == 0, hitIndex would become hitGlyphEnd.
+ This is not expected. It expects hitIndex < hitGlyphEnd if hitGlyphStart ≠ hitGlyphEnd.
+ Let hitIndex be hitGlyphStart-1 in the such condition.
+
+ Above change makes fast/text/ellipsis-text-rtl.html starting to
+ fail because offsetForPosition returns the character offset of the
+ next glyph if the argument 'h' is in a glyph boundary. In RTL
+ text, offsetForPosition should return a character offset of the
+ previous glyph in case of a glyph boundary. Use '<=' instead of '<'
+ for RTL text in order to select previous glyphs for glyph
+ boundaries.
+
+ Test: fast/dom/Document/CaretRangeFromPoint/rtl.html
+
+ * platform/graphics/ComplexTextController.cpp:
+ (WebCore::ComplexTextController::offsetForPosition): Compute correct hitGlyphEnd for RTL.
+
2020-01-08 Fujii Hironori <[email protected]>
[HarfBuzz][GTK] fast/text/complex-first-glyph-with-initial-advance.html is failing
Modified: releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/ComplexTextController.cpp (254911 => 254912)
--- releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/ComplexTextController.cpp 2020-01-22 10:42:29 UTC (rev 254911)
+++ releases/WebKitGTK/webkit-2.26/Source/WebCore/platform/graphics/ComplexTextController.cpp 2020-01-22 10:42:35 UTC (rev 254912)
@@ -205,7 +205,8 @@
for (unsigned j = 0; j < complexTextRun.glyphCount(); ++j) {
unsigned index = offsetIntoAdjustedGlyphs + j;
float adjustedAdvance = m_adjustedBaseAdvances[index].width();
- if (x < adjustedAdvance) {
+ bool hit = m_run.ltr() ? x < adjustedAdvance : (x <= adjustedAdvance && adjustedAdvance);
+ if (hit) {
unsigned hitGlyphStart = complexTextRun.indexAt(j);
unsigned hitGlyphEnd;
if (m_run.ltr())
@@ -215,7 +216,18 @@
// FIXME: Instead of dividing the glyph's advance equally between the characters, this
// could use the glyph's "ligature carets". This is available in CoreText via CTFontGetLigatureCaretPositions().
- unsigned hitIndex = hitGlyphStart + (hitGlyphEnd - hitGlyphStart) * (m_run.ltr() ? x / adjustedAdvance : 1 - x / adjustedAdvance);
+ unsigned hitIndex;
+ if (m_run.ltr())
+ hitIndex = hitGlyphStart + (hitGlyphEnd - hitGlyphStart) * (x / adjustedAdvance);
+ else {
+ if (hitGlyphStart == hitGlyphEnd)
+ hitIndex = hitGlyphStart;
+ else if (x)
+ hitIndex = hitGlyphEnd - (hitGlyphEnd - hitGlyphStart) * (x / adjustedAdvance);
+ else
+ hitIndex = hitGlyphEnd - 1;
+ }
+
unsigned stringLength = complexTextRun.stringLength();
CachedTextBreakIterator cursorPositionIterator(StringView(complexTextRun.characters(), stringLength), TextBreakIterator::Mode::Caret, nullAtom());
unsigned clusterStart;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
