[webkit-changes] [159076] trunk

Mon, 11 Nov 2013 15:29:33 -0800

Title: [159076] trunk
Revision
159076
Author
[email protected]
Date
2013-11-11 15:31:17 -0800 (Mon, 11 Nov 2013)

Log Message

[Mac] Characters too close together in complex Arabic text
https://bugs.webkit.org/show_bug.cgi?id=124057

Patch by Myles C. Maxfield <[email protected]> on 2013-11-11
Reviewed by Darin Adler.

Source/WebCore:

We weren't updating our total width variable with run's initial
advance information, leading to widths that were too narrow.

In addition, while initial advances for runs that aren't the first
run are accounted for by baking in the initial advances into the
previous character's advance, the initial advance for the first run
has to be accounted for in ComplexTextController::offsetForPosition.

Test: fast/text/complex-grapheme-cluster-with-initial-advance.html
Test: fast/text/selection-in-initial-advance-region.html

* platform/graphics/mac/ComplexTextController.cpp:
(WebCore::ComplexTextController::adjustGlyphsAndAdvances): Update
total width variable
(WebCore::ComplexTextController::offsetOfPosition): Account for
the first run's initial advance.

LayoutTests:

complex-grapheme-cluster-with-initial-advance adds a span around a word in some
complex Arabic text, and expects that the word spacing is the same as without the
span.

selection-in-initial-advance-region simulates a mouse drag across a complex text run
with an initial advance. This makes sure that ComplexTextController::offsetForPosition
doesn't crash when there is an initial advance.

* fast/text/complex-grapheme-cluster-with-initial-advance-expected.html: Added.
* fast/text/complex-grapheme-cluster-with-initial-advance.html: Added.
* fast/text/selection-in-initial-advance-region-expected.txt: added
* fast/text/selection-in-initial-advance-region.html: added

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (159075 => 159076)


--- trunk/LayoutTests/ChangeLog	2013-11-11 23:29:19 UTC (rev 159075)
+++ trunk/LayoutTests/ChangeLog	2013-11-11 23:31:17 UTC (rev 159076)
@@ -1,3 +1,23 @@
+2013-11-11  Myles C. Maxfield  <[email protected]>
+
+        [Mac] Characters too close together in complex Arabic text
+        https://bugs.webkit.org/show_bug.cgi?id=124057
+
+        Reviewed by Darin Adler.
+
+        complex-grapheme-cluster-with-initial-advance adds a span around a word in some
+        complex Arabic text, and expects that the word spacing is the same as without the
+        span.
+
+        selection-in-initial-advance-region simulates a mouse drag across a complex text run
+        with an initial advance. This makes sure that ComplexTextController::offsetForPosition
+        doesn't crash when there is an initial advance.
+
+        * fast/text/complex-grapheme-cluster-with-initial-advance-expected.html: Added.
+        * fast/text/complex-grapheme-cluster-with-initial-advance.html: Added.
+        * fast/text/selection-in-initial-advance-region-expected.txt: added
+        * fast/text/selection-in-initial-advance-region.html: added
+
 2013-11-11  Antti Koivisto  <[email protected]>
 
         End of line whitespace should collapse with white-space:pre-wrap; overflow-wrap:break-word in all cases

Added: trunk/LayoutTests/fast/text/complex-grapheme-cluster-with-initial-advance-expected.html (0 => 159076)


--- trunk/LayoutTests/fast/text/complex-grapheme-cluster-with-initial-advance-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/complex-grapheme-cluster-with-initial-advance-expected.html	2013-11-11 23:31:17 UTC (rev 159076)
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+<head><meta charset="utf-8"/></head>
+<body>
+<p>The first grapheme cluster here contains three code points: two characters (Lo unicode category) which join up to be drawn with a single glyph, and a third code point which is a nonspacing mark (Mn unicode category), which is drawn as a second glyph on top of the first glyph, using a negative advance. This run has an initial advance.</p>
+<div dir="RTL">
+<span style="font-size: 100pt; background: blue;">&#1604;&#1575;&#1611; <span>&#1609;</span> z</span></body>
+</div>
+</html>

Added: trunk/LayoutTests/fast/text/complex-grapheme-cluster-with-initial-advance.html (0 => 159076)


--- trunk/LayoutTests/fast/text/complex-grapheme-cluster-with-initial-advance.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/complex-grapheme-cluster-with-initial-advance.html	2013-11-11 23:31:17 UTC (rev 159076)
@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html>
+<head><meta charset="utf-8"/></head>
+<body>
+<p>The first grapheme cluster here contains three code points: two characters (Lo unicode category) which join up to be drawn with a single glyph, and a third code point which is a nonspacing mark (Mn unicode category), which is drawn as a second glyph on top of the first glyph, using a negative advance. This run has an initial advance.</p>
+<div dir="RTL">
+<span style="font-size: 100pt; background: blue;">&#1604;&#1575;&#1611; &#1609; z</span></body>
+</div>
+</html>

Added: trunk/LayoutTests/fast/text/selection-in-initial-advance-region-expected.txt (0 => 159076)


--- trunk/LayoutTests/fast/text/selection-in-initial-advance-region-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/text/selection-in-initial-advance-region-expected.txt	2013-11-11 23:31:17 UTC (rev 159076)
@@ -0,0 +1,8 @@
+This tests for a drag select crasher.
+
+لاً
+PASS sel.rangeCount is 1
+PASS r.startContainer is r.endContainer
+PASS r.startOffset is 0
+PASS r.endOffset is 3
+

Added: trunk/LayoutTests/fast/text/selection-in-initial-advance-region.html (0 => 159076)


--- trunk/LayoutTests/fast/text/selection-in-initial-advance-region.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/selection-in-initial-advance-region.html	2013-11-11 23:31:17 UTC (rev 159076)
@@ -0,0 +1,61 @@
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<p>This tests for a drag select crasher.</p>
+<div id="div" dir="RTL" style="-webkit-font-variant-ligatures: normal; -webkit-font-kerning: normal; font-size: 100px;"><span id="c" style="background: yellow;">&#1604;&#1575;&#1611;</span></div>
+<div id="console"></div>
+
+<script>
+var sel;
+var r;
+
+function log(str) {
+    var div = document.createElement("div");
+    div.appendChild(document.createTextNode(str));
+    var console = document.getElementById("console");
+    console.appendChild(div);
+}
+
+function test() {
+    if (window.eventSender && window.testRunner) {
+        testRunner.dumpAsText();
+        
+        var div = document.getElementById("div");
+        sel = window.getSelection();
+        sel.setPosition(div, 0);
+        
+        var start = document.getElementById("c");
+        var startx = start.offsetLeft + 1;
+        var starty = start.offsetTop + start.offsetHeight / 2;
+        eventSender.mouseMoveTo(startx, starty);
+        eventSender.mouseDown();
+    
+        var end = document.getElementById("c");
+        endx = end.offsetLeft + end.offsetWidth - 1;
+        endy = end.offsetTop + end.offsetHeight / 2;
+    
+        var steps = 20;
+        for (var i = 1; i <= steps; i++) {
+            eventSender.mouseMoveTo(startx + Math.abs(startx - endx) * (i / steps), starty + Math.abs(starty - endy) * (i / steps));
+        }
+    
+        eventSender.mouseMoveTo(endx, endy);
+        eventSender.mouseUp();
+    
+        sel = window.getSelection();
+        shouldBe("sel.rangeCount", "1");
+        r = sel.getRangeAt(0);
+        shouldBe("r.startContainer", "r.endContainer");
+        shouldBe("r.startOffset", "0");
+        shouldBe("r.endOffset", "3");
+    } else {
+        log("This uses the eventSender to perform a slow drag select.  To run it manually, click on the left half of the character and slowly drag to the right half of the character.");
+    }
+}
+
+test();
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (159075 => 159076)


--- trunk/Source/WebCore/ChangeLog	2013-11-11 23:29:19 UTC (rev 159075)
+++ trunk/Source/WebCore/ChangeLog	2013-11-11 23:31:17 UTC (rev 159076)
@@ -1,3 +1,27 @@
+2013-11-11  Myles C. Maxfield  <[email protected]>
+
+        [Mac] Characters too close together in complex Arabic text
+        https://bugs.webkit.org/show_bug.cgi?id=124057
+
+        Reviewed by Darin Adler.
+
+        We weren't updating our total width variable with run's initial
+        advance information, leading to widths that were too narrow.
+
+        In addition, while initial advances for runs that aren't the first
+        run are accounted for by baking in the initial advances into the
+        previous character's advance, the initial advance for the first run
+        has to be accounted for in ComplexTextController::offsetForPosition.
+
+        Test: fast/text/complex-grapheme-cluster-with-initial-advance.html
+        Test: fast/text/selection-in-initial-advance-region.html
+
+        * platform/graphics/mac/ComplexTextController.cpp:
+        (WebCore::ComplexTextController::adjustGlyphsAndAdvances): Update
+        total width variable
+        (WebCore::ComplexTextController::offsetOfPosition): Account for
+        the first run's initial advance.
+
 2013-11-11  Brady Eidson  <[email protected]>
 
         Make IDBBackingStoreTransaction be RefCounted

Modified: trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp (159075 => 159076)


--- trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp	2013-11-11 23:29:19 UTC (rev 159075)
+++ trunk/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp	2013-11-11 23:31:17 UTC (rev 159076)
@@ -188,7 +188,10 @@
     for (size_t r = 0; r < runCount; ++r) {
         const ComplexTextRun& complexTextRun = *m_complexTextRuns[r];
         for (unsigned j = 0; j < complexTextRun.glyphCount(); ++j) {
-            CGFloat adjustedAdvance = m_adjustedAdvances[offsetIntoAdjustedGlyphs + j].width;
+            size_t index = offsetIntoAdjustedGlyphs + j;
+            CGFloat adjustedAdvance = m_adjustedAdvances[index].width;
+            if (!index)
+                adjustedAdvance += complexTextRun.initialAdvance().width;
             if (x < adjustedAdvance) {
                 CFIndex hitGlyphStart = complexTextRun.indexAt(j);
                 CFIndex hitGlyphEnd;
@@ -578,6 +581,7 @@
             previousAdvance.height -= complexTextRun.initialAdvance().height;
             m_adjustedAdvances[m_adjustedAdvances.size() - 1] = previousAdvance;
         }
+        widthSinceLastCommit += complexTextRun.initialAdvance().width;
 
         if (!complexTextRun.isLTR())
             m_isLTROnly = false;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to