Attachment 28254 Details for Bug 24342 – A proposed fix

[patch] A proposed fix

issue24342-patch1.txt (text/plain), 16.09 KB, created by Hironori Bono on 2009-03-03 21:02:41 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Hironori Bono

Created: 2009-03-03 21:02:41 PST

Size: 16.09 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 41414)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,30 @@
>+2009-03-03  Hironori Bono  <hbono@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=24342
>+        Bug 24342: [Chromium] cannot insert a Thai character after a Thai
>+        prepend character
>+
>+        This change creates a new break iterator "codepointBreakIterator" for
>+        inputting characters.
>+        In "TextBreakIteratorICU.cpp", this break iterator uses custom rules
>+        based on the ones of ICU 3.8.
>+        On the other hand, in "TextBreakIteratorQt.cpp", this break iterator
>+        just calls the characterBreakIterator() function.
>+
>+        Test: editing/inserting/insert-thai-characters-001.html
>+
>+        * platform/text/TextBreakIterator.h: Added a new function codepointBreakIterator().
>+        * platform/text/TextBreakIteratorICU.cpp: Implemented the codepointBreakIterator() function for ICU.
>+        (WebCore::setUpIteratorWithRules): Ditto.
>+        (WebCore::codepointBreakIterator): Ditto.
>+        * platform/text/qt/TextBreakIteratorQt.cpp: Implemented the codepointBreakIterator() function for Qt.
>+        (WebCore::codepointBreakIterator): Ditto.
>+        * rendering/RenderText.cpp: Call the codepointBreakIterator() function when moving an input cursor.
>+        (WebCore::RenderText::previousOffset): Ditto.
>+        (WebCore::RenderText::nextOffset): Ditto.
>+
> 2009-03-03  Anders Carlsson  <andersca@apple.com>
> 
>         Reviewed by John Sullivan.
>Index: WebCore/platform/text/TextBreakIterator.h
>===================================================================
>--- WebCore/platform/text/TextBreakIterator.h	(revision 41414)
>+++ WebCore/platform/text/TextBreakIterator.h	(working copy)
>@@ -29,6 +29,9 @@ namespace WebCore {
>     class TextBreakIterator;
> 
>     // Note: The returned iterator is good only until you get another iterator.
>+#if PLATFORM(CHROMIUM)
>+    TextBreakIterator* codepointBreakIterator(const UChar*, int length);
>+#endif
>     TextBreakIterator* characterBreakIterator(const UChar*, int length);
>     TextBreakIterator* wordBreakIterator(const UChar*, int length);
>     TextBreakIterator* lineBreakIterator(const UChar*, int length);
>Index: WebCore/platform/text/TextBreakIteratorICU.cpp
>===================================================================
>--- WebCore/platform/text/TextBreakIteratorICU.cpp	(revision 41414)
>+++ WebCore/platform/text/TextBreakIteratorICU.cpp	(working copy)
>@@ -23,6 +23,7 @@
> #include "TextBreakIterator.h"
> 
> #include "TextBreakIteratorInternalICU.h"
>+#include "AtomicString.h"
> 
> #include <unicode/ubrk.h>
> #include <wtf/Assertions.h>
>@@ -114,4 +115,63 @@ bool isTextBreak(TextBreakIterator* bi, 
>     return ubrk_isBoundary(bi, pos);
> }
> 
>+static TextBreakIterator* setUpIteratorWithRules(bool& createdIterator, TextBreakIterator*& iterator,
>+    const char* breakRules, const UChar* string, int length)
>+{
>+    if (!string)
>+        return 0;
>+
>+    if (!createdIterator) {
>+        UParseError parseStatus;
>+        UErrorCode openStatus = U_ZERO_ERROR;
>+        AtomicString rules(breakRules);
>+        iterator = static_cast<TextBreakIterator*>(ubrk_openRules(rules.characters(), rules.length(), 0, 0, &parseStatus, &openStatus));
>+        createdIterator = true;
>+        ASSERT_WITH_MESSAGE(U_SUCCESS(openStatus), "ICU could not open a break iterator: %s (%d)", u_errorName(openStatus), openStatus);
>+    }
>+    if (!iterator)
>+      return 0;
>+
>+    UErrorCode setTextStatus = U_ZERO_ERROR;
>+    ubrk_setText(iterator, string, length, &setTextStatus);
>+    if (U_FAILURE(setTextStatus))
>+      return 0;
>+
>+    return iterator;
>+}
>+
>+TextBreakIterator* codepointBreakIterator(const UChar* string, int length)
>+{
>+    // This rule is based on the character-break iterator rules of ICU 3.8.
>+    // http://source.icu-project.org/repos/icu/icu/tags/release-3-8/source/data/brkitr/char.txt
>+    static const char* kRules =
>+        "$CR      = [\\p{Grapheme_Cluster_Break = CR}];"
>+        "$LF      = [\\p{Grapheme_Cluster_Break = LF}];"
>+        "$Control = [\\p{Grapheme_Cluster_Break = Control}];"
>+        "$VoiceMarks = [\\uff9e\\uff9f];"
>+        "$Extend  = [\\p{Grapheme_Cluster_Break = Extend} $VoiceMarks];"
>+        "$L       = [\\p{Grapheme_Cluster_Break = L}];"
>+        "$V       = [\\p{Grapheme_Cluster_Break = V}];"
>+        "$T       = [\\p{Grapheme_Cluster_Break = T}];"
>+        "$LV      = [\\p{Grapheme_Cluster_Break = LV}];"
>+        "$LVT     = [\\p{Grapheme_Cluster_Break = LVT}];"
>+        "$HangulSyllable = $L+ | ($L* ($LV? $V+ | $LV | $LVT) $T*) | $T+;"
>+        "!!chain;"
>+        "!!forward;"
>+        "$CR $LF;"
>+        "([^$Control $CR $LF] | $HangulSyllable) $Extend*;"
>+        "!!reverse;"
>+        "$BackHangulSyllable = $L+ | ($T* ($V+$LV? | $LV | $LVT) $L*) | $T+;"
>+        "$BackOneCluster = ($LF $CR) | ($Extend* ([^$Control $CR $LF] | $BackHangulSyllable));"
>+        "$BackOneCluster;"
>+        "!!safe_reverse;"
>+        "$V+ $L;"
>+        "!!safe_forward;"
>+        "$V+ $T;";
>+    static bool createdCodepointBreakIterator = false;
>+    static TextBreakIterator* staticCodepointBreakIterator;
>+    return setUpIteratorWithRules(createdCodepointBreakIterator,
>+        staticCodepointBreakIterator, kRules, string, length);
>+}
>+
> }
>Index: WebCore/platform/text/qt/TextBreakIteratorQt.cpp
>===================================================================
>--- WebCore/platform/text/qt/TextBreakIteratorQt.cpp	(revision 41414)
>+++ WebCore/platform/text/qt/TextBreakIteratorQt.cpp	(working copy)
>@@ -63,6 +63,11 @@ namespace WebCore {
>         return static_cast<TextBreakIterator*>(iterator);
>     }
> 
>+    TextBreakIterator* codepointBreakIterator(const UChar* string, int length)
>+    {
>+        return characterBreakIterator(string, length);
>+    }
>+
>     TextBreakIterator* lineBreakIterator(const UChar* string, int length)
>     {
>         static QTextBoundaryFinder *iterator = 0;
>@@ -250,6 +255,11 @@ TextBreakIterator* characterBreakIterato
>     return iterator;
> }
> 
>+TextBreakIterator* codepointBreakIterator(const UChar* string, int length)
>+{
>+    return characterBreakIterator(string, length);
>+}
>+
> TextBreakIterator* lineBreakIterator(const UChar*, int)
> {
>     // not yet implemented
>Index: WebCore/rendering/RenderText.cpp
>===================================================================
>--- WebCore/rendering/RenderText.cpp	(revision 41414)
>+++ WebCore/rendering/RenderText.cpp	(working copy)
>@@ -1147,7 +1147,7 @@ unsigned RenderText::caretMaxRenderedOff
> int RenderText::previousOffset(int current) const
> {
>     StringImpl* si = m_text.get();
>-    TextBreakIterator* iterator = characterBreakIterator(si->characters(), si->length());
>+    TextBreakIterator* iterator = codepointBreakIterator(si->characters(), si->length());
>     if (!iterator)
>         return current - 1;
> 
>@@ -1270,7 +1270,7 @@ int RenderText::previousOffsetForBackwar
> int RenderText::nextOffset(int current) const
> {
>     StringImpl* si = m_text.get();
>-    TextBreakIterator* iterator = characterBreakIterator(si->characters(), si->length());
>+    TextBreakIterator* iterator = codepointBreakIterator(si->characters(), si->length());
>     if (!iterator)
>         return current + 1;
> 
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 41414)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2009-03-03  Hironori Bono  <hbono@chromium.org>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=24342
>+        Bug 24342: [Chromium] cannot insert a Thai character after a Thai
>+        prepend character
>+
>+        Add a test that verifies we can insert a character after a Thai prepend character.
>+        
>+        * editing/inserting/insert-thai-characters-001-expected.txt: Added.
>+        * editing/inserting/insert-thai-characters-001.html: Added.
>+
> 2009-03-03  Anders Carlsson  <andersca@apple.com>
> 
>         Reviewed by John Sullivan.
>Index: LayoutTests/editing/inserting/insert-thai-characters-001-expected.txt
>===================================================================
>--- LayoutTests/editing/inserting/insert-thai-characters-001-expected.txt	(revision 0)
>+++ LayoutTests/editing/inserting/insert-thai-characters-001-expected.txt	(revision 0)
>@@ -0,0 +1,50 @@
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV to 0 of DIV toDOMRange:range from 1 of #text > DIV to 1 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > DIV to 1 of #text > DIV toDOMRange:range from 2 of #text > DIV to 2 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 2 of #text > DIV to 2 of #text > DIV toDOMRange:range from 3 of #text > DIV to 3 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 3 of #text > DIV to 3 of #text > DIV toDOMRange:range from 4 of #text > DIV to 4 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 4 of #text > DIV to 4 of #text > DIV toDOMRange:range from 5 of #text > DIV to 5 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > DIV to 5 of #text > DIV toDOMRange:range from 6 of #text > DIV to 6 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 6 of #text > DIV to 6 of #text > DIV toDOMRange:range from 7 of #text > DIV to 7 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 7 of #text > DIV to 7 of #text > DIV toDOMRange:range from 8 of #text > DIV to 8 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 8 of #text > DIV to 8 of #text > DIV toDOMRange:range from 9 of #text > DIV to 9 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 9 of #text > DIV to 9 of #text > DIV toDOMRange:range from 10 of #text > DIV to 10 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 10 of #text > DIV to 10 of #text > DIV toDOMRange:range from 11 of #text > DIV to 11 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 4 of #text > DIV to 4 of #text > DIV toDOMRange:range from 5 of #text > DIV to 5 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > DIV to 5 of #text > DIV toDOMRange:range from 6 of #text > DIV to 6 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
>+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
>+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
>+This test tests whether we can insert a Thai character after a Thai "prepend" character.
>+
>+If this test succeeds, you can see a string "succeeded" below.
>+
>+
>+Succeeded.
>Index: LayoutTests/editing/inserting/insert-thai-characters-001.html
>===================================================================
>--- LayoutTests/editing/inserting/insert-thai-characters-001.html	(revision 0)
>+++ LayoutTests/editing/inserting/insert-thai-characters-001.html	(revision 0)
>@@ -0,0 +1,50 @@
>+<html>
>+<head>
>+<title>Editing Test (Inserting a Thai character after a Thai prepend character)</title>
>+<script src="../editing.js" language="javascript" type="text/javascript"></script>
>+<script language="javascript" type="text/javascript">
>+function log(str) {
>+    var li = document.createElement("li");
>+    li.appendChild(document.createTextNode(str));
>+    var console = document.getElementById("console");
>+    console.appendChild(li);
>+}
>+function editingTest() {
>+    if (window.layoutTestController)
>+        window.layoutTestController.dumpAsText();
>+    var textarea = document.getElementById("test");
>+    textarea.focus();
>+    typeCharacterCommand(String.fromCharCode(0x0E2D));
>+    typeCharacterCommand(String.fromCharCode(0x0E22));
>+    typeCharacterCommand(String.fromCharCode(0x0E32));
>+    typeCharacterCommand(String.fromCharCode(0x0E01));
>+    typeCharacterCommand(String.fromCharCode(0x0E0D));
>+    typeCharacterCommand(String.fromCharCode(0x0E35));
>+    typeCharacterCommand(String.fromCharCode(0x0E48));
>+    typeCharacterCommand(String.fromCharCode(0x0E1B));
>+    typeCharacterCommand(String.fromCharCode(0x0E38));
>+    typeCharacterCommand(String.fromCharCode(0x0E48));
>+    typeCharacterCommand(String.fromCharCode(0x0E19));
>+    moveSelectionBackwardByCharacterCommand();
>+    moveSelectionBackwardByCharacterCommand();
>+    moveSelectionBackwardByCharacterCommand();
>+    typeCharacterCommand(String.fromCharCode(0x0E44));
>+    typeCharacterCommand(String.fromCharCode(0x0E1B));
>+    var expected_result = "\u0E2D\u0E22\u0E32\u0E01\u0E44\u0E1B\u0E0D\u0E35\u0E48\u0E1B\u0E38\u0E48\u0E19";
>+    if (textarea.value == expected_result)
>+        log("Succeeded.");
>+     else
>+        log("Failed. Actual: \"" + textarea.value + "\", Expected: \"" + expected_result + "\".");
>+}
>+</script>
>+</head>
>+<body>
>+<p>This test tests whether we can insert a Thai character after a Thai "prepend" character.</p>
>+<p>If this test succeeds, you can see a string "succeeded" below.</p>
>+<textarea id="test" rows="1" cols="40"></textarea>
>+<ul id="console"></ul>
>+<script language="javascript" type="text/javascript">
>+runEditingTest();
>+</script>
>+</body>
>+</html>
>
>Property changes on: LayoutTests/editing/inserting/insert-thai-characters-001.html
>___________________________________________________________________
>Name: svn:executable
>   + *
>

Flags:

ap: review-

Actions: View | Formatted Diff | Diff

Attachments on bug 24342: 28254 | 28262 | 28346