Fix ShapeResult::selectionRect handles RTL runs in wrong order

third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp

Index: third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
index 64de83df863e65d442e72f4a3e11af7e232ba927..35b6f313157209e6ad765adda4bc963d22f3c9b9 100644
--- a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
+++ b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaper.cpp
@@ -75,6 +75,8 @@ struct ShapeResult::RunInfo {
         m_glyphData.resize(m_numGlyphs);
     }
 
+    bool rtl() const { return HB_DIRECTION_IS_BACKWARD(m_direction); }
+    float xPositionForVisualOffset(unsigned) const;
     float xPositionForOffset(unsigned) const;
     int characterIndexForXPosition(float) const;
     void setGlyphAndPositions(unsigned index, uint16_t glyphId, float advance,
@@ -100,12 +102,20 @@ struct ShapeResult::RunInfo {
     float m_width;
 };
 
+float ShapeResult::RunInfo::xPositionForVisualOffset(unsigned offset) const
+{
+    ASSERT(offset < m_numCharacters);
+    if (rtl())
+        offset = m_numCharacters - offset - 1;
+    return xPositionForOffset(offset);
+}
+
 float ShapeResult::RunInfo::xPositionForOffset(unsigned offset) const
 {
     ASSERT(offset <= m_numCharacters);
     unsigned glyphIndex = 0;
     float position = 0;
-    if (m_direction == HB_DIRECTION_RTL) {
+    if (rtl()) {
         while (glyphIndex < m_numGlyphs && m_glyphData[glyphIndex].characterIndex > offset) {
             position += m_glyphData[glyphIndex].advance;
             ++glyphIndex;
@@ -138,7 +148,7 @@ int ShapeResult::RunInfo::characterIndexForXPosition(float targetX) const
         currentAdvance += m_glyphData[++glyphIndex].advance;
     currentAdvance = currentAdvance / 2.0;
     if (targetX <= currentAdvance)
-        return m_direction == HB_DIRECTION_RTL ? m_numCharacters : 0;
+        return rtl() ? m_numCharacters : 0;
 
     currentX = currentAdvance;
     ++glyphIndex;
@@ -151,12 +161,12 @@ int ShapeResult::RunInfo::characterIndexForXPosition(float targetX) const
         currentAdvance = currentAdvance / 2.0;
         float nextX = currentX + prevAdvance + currentAdvance;
         if (currentX <= targetX && targetX <= nextX)
-            return m_direction == HB_DIRECTION_RTL ? prevCharacterIndex : m_glyphData[glyphIndex].characterIndex;
+            return rtl() ? prevCharacterIndex : m_glyphData[glyphIndex].characterIndex;
         currentX = nextX;
         ++glyphIndex;
     }
 
-    return m_direction == HB_DIRECTION_RTL ? 0 : m_numCharacters;
+    return rtl() ? 0 : m_numCharacters;
 }
 
 void ShapeResult::RunInfo::setGlyphAndPositions(unsigned index,
@@ -410,21 +420,29 @@ FloatRect ShapeResult::selectionRect(Vector<RefPtr<ShapeResult>>& results,
     unsigned totalNumCharacters = 0;
     for (unsigned j = 0; j < results.size(); j++) {
         RefPtr<ShapeResult> result = results[j];
+        if (direction == RTL) {
+            // Convert logical offsets to visual offsets, because results are in

[wkorman, 2015/11/02 04:29:20]

Is visual the same as physical? Which name is preferred?

[kojii, 2015/11/02 05:19:44]

IIUC Unicode bidi reordering uses logical/visual while CSS directional
properties use logical/physical. Existing comments in HarfBuzzShaper is
consistent on that.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

+            // logical order while runs are in visual order.
+            if (!foundFromX && from >= 0 && static_cast<unsigned>(from) < result->numCharacters())
+                from = result->numCharacters() - from - 1;
+            if (!foundToX && to >= 0 && static_cast<unsigned>(to) < result->numCharacters())
+                to = result->numCharacters() - to - 1;
+            currentX -= result->width();
+        }
         for (unsigned i = 0; i < result->m_runs.size(); i++) {
             if (!result->m_runs[i])
                 continue;
-            if (direction == RTL)
-                currentX -= result->m_runs[i]->m_width;
+            ASSERT((direction == RTL) == result->m_runs[i]->rtl());
             int numCharacters = result->m_runs[i]->m_numCharacters;
             if (!foundFromX && from >= 0 && from < numCharacters) {
-                fromX = result->m_runs[i]->xPositionForOffset(from) + currentX;
+                fromX = result->m_runs[i]->xPositionForVisualOffset(from) + currentX;
                 foundFromX = true;
             } else {
                 from -= numCharacters;
             }
 
             if (!foundToX && to >= 0 && to < numCharacters) {
-                toX = result->m_runs[i]->xPositionForOffset(to) + currentX;
+                toX = result->m_runs[i]->xPositionForVisualOffset(to) + currentX;
                 foundToX = true;
             } else {
                 to -= numCharacters;
@@ -432,9 +450,10 @@ FloatRect ShapeResult::selectionRect(Vector<RefPtr<ShapeResult>>& results,
 
             if (foundFromX && foundToX)
                 break;
-            if (direction != RTL)
-                currentX += result->m_runs[i]->m_width;
+            currentX += result->m_runs[i]->m_width;
         }
+        if (direction == RTL)
+            currentX -= result->width();
         totalNumCharacters += result->numCharacters();
     }