2011-01-25 Ned Holbrook <nholbrook@apple.com>

        Reviewed by Dan Bernstein.

        ComplexTextController incorrectly conflates string length and range of indexes
        https://bugs.webkit.org/show_bug.cgi?id=52760

        * fast/text/offsetForPosition-complex-fallback-expected.txt: Added.
        * fast/text/offsetForPosition-complex-fallback.html: Added.
2011-01-25  Ned Holbrook  <nholbrook@apple.com>

        Reviewed by Dan Bernstein.

        ComplexTextController incorrectly conflates string length and range of indexes
        https://bugs.webkit.org/show_bug.cgi?id=52760

        Test: fast/text/offsetForPosition-complex-fallback.html

        * platform/graphics/mac/ComplexTextController.cpp:
        (WebCore::ComplexTextController::offsetForPosition):
        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
        (WebCore::ComplexTextController::ComplexTextRun::setIsNonMonotonic):
        (WebCore::ComplexTextController::advance):
        * platform/graphics/mac/ComplexTextController.h:
        (WebCore::ComplexTextController::ComplexTextRun::create):
        (WebCore::ComplexTextController::ComplexTextRun::indexEnd):
        * platform/graphics/mac/ComplexTextControllerATSUI.cpp:
        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
        * platform/graphics/mac/ComplexTextControllerCoreText.cpp:
        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
        (WebCore::ComplexTextController::collectComplexTextRunsForCharactersCoreText):

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index f5a3708..5b55410 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2011-01-25  Ned Holbrook  <nholbrook@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        ComplexTextController incorrectly conflates string length and range of indexes
+        https://bugs.webkit.org/show_bug.cgi?id=52760
+
+        * fast/text/offsetForPosition-complex-fallback-expected.txt: Added.
+        * fast/text/offsetForPosition-complex-fallback.html: Added.
+
 2011-01-25  Sam Weinig  <sam@webkit.org>
 
         Make this test a bit more robust agains different scrollbar widths.

diff --git a/LayoutTests/fast/text/offsetForPosition-complex-fallback-expected.txt b/LayoutTests/fast/text/offsetForPosition-complex-fallback-expected.txt
new file mode 100644 (file)
index 0000000..a00ff4d
--- /dev/null
+++ b/LayoutTests/fast/text/offsetForPosition-complex-fallback-expected.txt
@@ -0,0 +1,5 @@
+Test offset for position computation across complex runs with font fallbacks, in this case due to a font (Arial) lacking a newline glyph.
+
+PASS
+
+Jus­ti­fi­ca­tion should be used sparingly and cautiously on Web pages.

diff --git a/LayoutTests/fast/text/offsetForPosition-complex-fallback.html b/LayoutTests/fast/text/offsetForPosition-complex-fallback.html
new file mode 100644 (file)
index 0000000..5bf2400
--- /dev/null
+++ b/LayoutTests/fast/text/offsetForPosition-complex-fallback.html
@@ -0,0 +1,37 @@
+<p>
+    Test offset for position computation across complex runs with font
+    fallbacks, in this case due to a font (Arial) lacking a newline glyph.
+</p>
+<p id="result">
+    Test did not run.
+</p>
+<div id="target" style="display: inline; font-family: Arial; text-rendering: optimizeLegibility;">
+Jus&shy;ti&shy;fi&shy;ca&shy;tion
+should
+be used sparingly and cautiously on Web pages.
+</div>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+
+        var target = document.getElementById("target");
+        var result = document.getElementById("result");
+        var x = target.offsetLeft + target.offsetWidth / 2;
+        var y = target.offsetTop + target.offsetHeight / 2;
+
+        eventSender.leapForward(1000);
+        eventSender.mouseMoveTo(x, y);
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+        eventSender.leapForward(1);
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+        eventSender.leapForward(1000);
+
+        var selectedText = getSelection().toString();
+        if (selectedText == "sparingly")
+            result.innerText = "PASS";
+        else
+            result.innerText = "FAIL: Selected text is \"" + selectedText + "\"";
+    }
+</script>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 186a1c4..b9a714f 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2011-01-25  Ned Holbrook  <nholbrook@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        ComplexTextController incorrectly conflates string length and range of indexes
+        https://bugs.webkit.org/show_bug.cgi?id=52760
+
+        Test: fast/text/offsetForPosition-complex-fallback.html
+
+        * platform/graphics/mac/ComplexTextController.cpp:
+        (WebCore::ComplexTextController::offsetForPosition):
+        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
+        (WebCore::ComplexTextController::ComplexTextRun::setIsNonMonotonic):
+        (WebCore::ComplexTextController::advance):
+        * platform/graphics/mac/ComplexTextController.h:
+        (WebCore::ComplexTextController::ComplexTextRun::create):
+        (WebCore::ComplexTextController::ComplexTextRun::indexEnd):
+        * platform/graphics/mac/ComplexTextControllerATSUI.cpp:
+        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
+        * platform/graphics/mac/ComplexTextControllerCoreText.cpp:
+        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun):
+        (WebCore::ComplexTextController::collectComplexTextRunsForCharactersCoreText):
+
 2011-01-25  Sam Weinig  <sam@webkit.org>
 
         Reviewed by David Hyatt.

diff --git a/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp b/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp
index 86f6bec..e58f703 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp
+++ b/Source/WebCore/platform/graphics/mac/ComplexTextController.cpp
@@ -118,9 +118,9 @@ int ComplexTextController::offsetForPosition(float h, bool includePartialGlyphs)
                 CFIndex hitGlyphStart = complexTextRun.indexAt(j);
                 CFIndex hitGlyphEnd;
                 if (m_run.ltr())
-                    hitGlyphEnd = max<CFIndex>(hitGlyphStart, j + 1 < complexTextRun.glyphCount() ? complexTextRun.indexAt(j + 1) : static_cast<CFIndex>(complexTextRun.stringLength()));
+                    hitGlyphEnd = max<CFIndex>(hitGlyphStart, j + 1 < complexTextRun.glyphCount() ? complexTextRun.indexAt(j + 1) : static_cast<CFIndex>(complexTextRun.indexEnd()));
                 else
-                    hitGlyphEnd = max<CFIndex>(hitGlyphStart, j > 0 ? complexTextRun.indexAt(j - 1) : static_cast<CFIndex>(complexTextRun.stringLength()));
+                    hitGlyphEnd = max<CFIndex>(hitGlyphStart, j > 0 ? complexTextRun.indexAt(j - 1) : static_cast<CFIndex>(complexTextRun.indexEnd()));
 
                 // FIXME: Instead of dividing the glyph's advance equally between the characters, this
                 // could use the glyph's "ligature carets". However, there is no Core Text API to get the
@@ -311,6 +311,7 @@ ComplexTextController::ComplexTextRun::ComplexTextRun(const SimpleFontData* font
     , m_characters(characters)
     , m_stringLocation(stringLocation)
     , m_stringLength(stringLength)
+    , m_indexEnd(stringLocation + stringLength)
     , m_isMonotonic(true)
 {
 #if USE(CORE_TEXT) && USE(ATSUI)
@@ -335,7 +336,7 @@ void ComplexTextController::ComplexTextRun::setIsNonMonotonic()
 
     m_glyphEndOffsets.grow(m_glyphCount);
     for (size_t i = 0; i < m_glyphCount; ++i) {
-        CFIndex nextMappedIndex = m_stringLength;
+        CFIndex nextMappedIndex = m_indexEnd;
         for (size_t j = indexAt(i) + 1; j < m_stringLength; ++j) {
             if (mappedIndices[j]) {
                 nextMappedIndex = j;
@@ -370,9 +371,9 @@ void ComplexTextController::advance(unsigned offset, GlyphBuffer* glyphBuffer)
             unsigned glyphEndOffset;
             if (complexTextRun.isMonotonic()) {
                 if (ltr)
-                    glyphEndOffset = max<unsigned>(glyphStartOffset, g + 1 < glyphCount ? static_cast<unsigned>(complexTextRun.indexAt(g + 1)) : complexTextRun.stringLength());
+                    glyphEndOffset = max<unsigned>(glyphStartOffset, g + 1 < glyphCount ? static_cast<unsigned>(complexTextRun.indexAt(g + 1)) : complexTextRun.indexEnd());
                 else
-                    glyphEndOffset = max<unsigned>(glyphStartOffset, g > 0 ? static_cast<unsigned>(complexTextRun.indexAt(g - 1)) : complexTextRun.stringLength());
+                    glyphEndOffset = max<unsigned>(glyphStartOffset, g > 0 ? static_cast<unsigned>(complexTextRun.indexAt(g - 1)) : complexTextRun.indexEnd());
             } else
                 glyphEndOffset = complexTextRun.endOffsetAt(g);
 
@@ -386,7 +387,7 @@ void ComplexTextController::advance(unsigned offset, GlyphBuffer* glyphBuffer)
 
             unsigned oldCharacterInCurrentGlyph = m_characterInCurrentGlyph;
             m_characterInCurrentGlyph = min(m_currentCharacter - complexTextRun.stringLocation(), glyphEndOffset) - glyphStartOffset;
-            // FIXME: Instead of dividing the glyph's advance equially between the characters, this
+            // FIXME: Instead of dividing the glyph's advance equally between the characters, this
             // could use the glyph's "ligature carets". However, there is no Core Text API to get the
             // ligature carets.
             if (glyphStartOffset == glyphEndOffset) {

diff --git a/Source/WebCore/platform/graphics/mac/ComplexTextController.h b/Source/WebCore/platform/graphics/mac/ComplexTextController.h
index 9cf80a6..f97dc92 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/ComplexTextController.h
+++ b/Source/WebCore/platform/graphics/mac/ComplexTextController.h
@@ -72,9 +72,9 @@ private:
     class ComplexTextRun : public RefCounted<ComplexTextRun> {
     public:
 #if USE(CORE_TEXT)
-        static PassRefPtr<ComplexTextRun> create(CTRunRef ctRun, const SimpleFontData* fontData, const UChar* characters, unsigned stringLocation, size_t stringLength)
+        static PassRefPtr<ComplexTextRun> create(CTRunRef ctRun, const SimpleFontData* fontData, const UChar* characters, unsigned stringLocation, size_t stringLength, CFRange runRange)
         {
-            return adoptRef(new ComplexTextRun(ctRun, fontData, characters, stringLocation, stringLength));
+            return adoptRef(new ComplexTextRun(ctRun, fontData, characters, stringLocation, stringLength, runRange));
         }
 #endif
 #if USE(ATSUI)
@@ -94,6 +94,7 @@ private:
         unsigned stringLocation() const { return m_stringLocation; }
         size_t stringLength() const { return m_stringLength; }
         ALWAYS_INLINE CFIndex indexAt(size_t i) const;
+        CFIndex indexEnd() const { return m_indexEnd; }
         CFIndex endOffsetAt(size_t i) const { ASSERT(!m_isMonotonic); return m_glyphEndOffsets[i]; }
         const CGGlyph* glyphs() const { return m_glyphs; }
         const CGSize* advances() const { return m_advances; }
@@ -102,7 +103,7 @@ private:
 
     private:
 #if USE(CORE_TEXT)
-        ComplexTextRun(CTRunRef, const SimpleFontData*, const UChar* characters, unsigned stringLocation, size_t stringLength);
+        ComplexTextRun(CTRunRef, const SimpleFontData*, const UChar* characters, unsigned stringLocation, size_t stringLength, CFRange runRange);
         void createTextRunFromFontDataCoreText(bool ltr);
 #endif
 #if USE(ATSUI)
@@ -133,6 +134,7 @@ private:
 #if USE(ATSUI)
         Vector<CFIndex, 64> m_atsuiIndices;
 #endif
+        CFIndex m_indexEnd;
         Vector<CFIndex, 64> m_glyphEndOffsets;
         Vector<CGGlyph, 64> m_glyphsVector;
         const CGGlyph* m_glyphs;

diff --git a/Source/WebCore/platform/graphics/mac/ComplexTextControllerATSUI.cpp b/Source/WebCore/platform/graphics/mac/ComplexTextControllerATSUI.cpp
index 9c2ab6b..a0b58aa 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/ComplexTextControllerATSUI.cpp
+++ b/Source/WebCore/platform/graphics/mac/ComplexTextControllerATSUI.cpp
@@ -146,6 +146,7 @@ ComplexTextController::ComplexTextRun::ComplexTextRun(ATSUTextLayout atsuTextLay
     , m_characters(characters)
     , m_stringLocation(stringLocation)
     , m_stringLength(stringLength)
+    , m_indexEnd(stringLocation + stringLength)
     , m_directionalOverride(directionalOverride)
     , m_isMonotonic(true)
 {

diff --git a/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp b/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp
index 07fb153..239113f 100644 (file)
--- a/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp
+++ b/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp
@@ -42,12 +42,13 @@ extern const CFStringRef kCTTypesetterOptionForcedEmbeddingLevel;
 
 namespace WebCore {
 
-ComplexTextController::ComplexTextRun::ComplexTextRun(CTRunRef ctRun, const SimpleFontData* fontData, const UChar* characters, unsigned stringLocation, size_t stringLength)
+ComplexTextController::ComplexTextRun::ComplexTextRun(CTRunRef ctRun, const SimpleFontData* fontData, const UChar* characters, unsigned stringLocation, size_t stringLength, CFRange runRange)
     : m_coreTextRun(ctRun)
     , m_fontData(fontData)
     , m_characters(characters)
     , m_stringLocation(stringLocation)
     , m_stringLength(stringLength)
+    , m_indexEnd(runRange.location + runRange.length)
     , m_isMonotonic(true)
 {
     m_glyphCount = CTRunGetGlyphCount(m_coreTextRun.get());
@@ -165,7 +166,8 @@ void ComplexTextController::collectComplexTextRunsForCharactersCoreText(const UC
     for (CFIndex r = 0; r < runCount; r++) {
         CTRunRef ctRun = static_cast<CTRunRef>(CFArrayGetValueAtIndex(runArray, r));
         ASSERT(CFGetTypeID(ctRun) == CTRunGetTypeID());
-        m_complexTextRuns.append(ComplexTextRun::create(ctRun, fontData, cp, stringLocation, length));
+        CFRange runRange = CTRunGetStringRange(ctRun);
+        m_complexTextRuns.append(ComplexTextRun::create(ctRun, fontData, cp, stringLocation, length, runRange));
     }
 }