Changeset 73548 in webkit


Ignore:
Timestamp:
Dec 8, 2010 1:55:54 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

2010-12-08 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Dan Bernstein.

REGRESSION: LayoutTests/editing/selection/caret-rtl-2.html fails
​https://bugs.webkit.org/show_bug.cgi?id=33503

The bug was caused by Font::offsetForPosition's not taking into account the containing block's text direction.
When RTL text appears in a LTR block, the offset at the beginning of RTL text is on the left of RTL text,
adn the offset at the end of RTL text is on the right of RTL text. For example, if we had RTL text CBA,
then the correspondance between letters and offsets in logical order are: A -> 0, B -> 1, and C -> 2.

Case 1. CBA appears in a RTL block:

In this case, clicking on the visual left of CBA puts the caret naturally at Position("CBA", 2).
Clicking on the visual right of CBA puts the caret at Position("CBA", 0) as expected.

Case 2. CBA appears in a LTR block:

Because the containing block flows from left to right, by covention, Position("CBA", 2") coresponds
to the visual right of CBA, and Position("CBA", 0) corresponds to the visual left of CBA.
Therefore, clicking on the visual left of CBA should put the caret at Positoin("CBA", 0),
and clicking on the visual right should put it at Position("CBA", 2).

The bug was caused by WebKit's not considering case 2. The same bug also existe for LTR text in a RTL block.
Fixed the bug by taking care of the case 2 in InlineTextBox::offsetForPosition.

Tests: editing/selection/caret-ltr-2-left.html

editing/selection/caret-ltr-2.html
editing/selection/caret-ltr-right.html
editing/selection/caret-ltr.html
editing/selection/caret-rtl-2-left.html
editing/selection/caret-rtl-right.html

  • rendering/InlineTextBox.cpp: (WebCore::InlineTextBox::offsetForPosition):

2010-12-08 Ryosuke Niwa <​rniwa@webkit.org>

Reviewed by Dan Bernstein.

REGRESSION: LayoutTests/editing/selection/caret-rtl-2.html fails
​https://bugs.webkit.org/show_bug.cgi?id=33503

Added tests to ensure WebKit places the caret at a right place for LTR text in a LTR block,
LTR text in a RTL block, RTL text in a RTL block, and RTL text in a LTR block.

Because we need to verify both rendering of the caret and (node, offset) pair of the caret,
these tests cannot be combined or done without pixel results.

  • editing/selection/caret-ltr-2-left.html: Added.
  • editing/selection/caret-ltr-2.html: Added.
  • editing/selection/caret-ltr-right.html: Added.
  • editing/selection/caret-ltr.html: Added.
  • editing/selection/caret-rtl-2-left.html: Added.
  • editing/selection/caret-rtl-2.html:
  • editing/selection/caret-rtl-right.html: Added.
  • editing/selection/caret-rtl.html:
  • editing/selection/resources/caret-edge-shared.js: Added. (runTest): (verify): (log):
  • platform/mac/editing/selection/caret-ltr-2-expected.checksum: Added.
  • platform/mac/editing/selection/caret-ltr-2-expected.png: Added.
  • platform/mac/editing/selection/caret-ltr-2-expected.txt: Added.
  • platform/mac/editing/selection/caret-ltr-2-left-expected.checksum: Added.
  • platform/mac/editing/selection/caret-ltr-2-left-expected.png: Added.
  • platform/mac/editing/selection/caret-ltr-2-left-expected.txt: Added.
  • platform/mac/editing/selection/caret-ltr-expected.checksum: Added.
  • platform/mac/editing/selection/caret-ltr-expected.png: Added.
  • platform/mac/editing/selection/caret-ltr-expected.txt: Added.
  • platform/mac/editing/selection/caret-ltr-right-expected.checksum: Added.
  • platform/mac/editing/selection/caret-ltr-right-expected.png: Added.
  • platform/mac/editing/selection/caret-ltr-right-expected.txt: Added.
  • platform/mac/editing/selection/caret-rtl-2-expected.checksum:
  • platform/mac/editing/selection/caret-rtl-2-expected.png:
  • platform/mac/editing/selection/caret-rtl-2-expected.txt:
  • platform/mac/editing/selection/caret-rtl-2-left-expected.checksum: Added.
  • platform/mac/editing/selection/caret-rtl-2-left-expected.png: Added.
  • platform/mac/editing/selection/caret-rtl-2-left-expected.txt: Added.
  • platform/mac/editing/selection/caret-rtl-expected.checksum:
  • platform/mac/editing/selection/caret-rtl-expected.png:
  • platform/mac/editing/selection/caret-rtl-expected.txt:
  • platform/mac/editing/selection/caret-rtl-right-expected.checksum: Added.
  • platform/mac/editing/selection/caret-rtl-right-expected.png: Added.
  • platform/mac/editing/selection/caret-rtl-right-expected.txt: Added.
Location:
trunk
Files:
25 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r73547 r73548  
     12010-12-08  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Dan Bernstein.
     4
     5        REGRESSION: LayoutTests/editing/selection/caret-rtl-2.html fails
     6        https://bugs.webkit.org/show_bug.cgi?id=33503
     7
     8        Added tests to ensure WebKit places the caret at a right place for LTR text in a LTR block,
     9        LTR text in a RTL block, RTL text in a RTL block, and RTL text in a LTR block.
     10
     11        Because we need to verify both rendering of the caret and (node, offset) pair of the caret,
     12        these tests cannot be combined or done without pixel results.
     13
     14        * editing/selection/caret-ltr-2-left.html: Added.
     15        * editing/selection/caret-ltr-2.html: Added.
     16        * editing/selection/caret-ltr-right.html: Added.
     17        * editing/selection/caret-ltr.html: Added.
     18        * editing/selection/caret-rtl-2-left.html: Added.
     19        * editing/selection/caret-rtl-2.html:
     20        * editing/selection/caret-rtl-right.html: Added.
     21        * editing/selection/caret-rtl.html:
     22        * editing/selection/resources/caret-edge-shared.js: Added.
     23        (runTest):
     24        (verify):
     25        (log):
     26        * platform/mac/editing/selection/caret-ltr-2-expected.checksum: Added.
     27        * platform/mac/editing/selection/caret-ltr-2-expected.png: Added.
     28        * platform/mac/editing/selection/caret-ltr-2-expected.txt: Added.
     29        * platform/mac/editing/selection/caret-ltr-2-left-expected.checksum: Added.
     30        * platform/mac/editing/selection/caret-ltr-2-left-expected.png: Added.
     31        * platform/mac/editing/selection/caret-ltr-2-left-expected.txt: Added.
     32        * platform/mac/editing/selection/caret-ltr-expected.checksum: Added.
     33        * platform/mac/editing/selection/caret-ltr-expected.png: Added.
     34        * platform/mac/editing/selection/caret-ltr-expected.txt: Added.
     35        * platform/mac/editing/selection/caret-ltr-right-expected.checksum: Added.
     36        * platform/mac/editing/selection/caret-ltr-right-expected.png: Added.
     37        * platform/mac/editing/selection/caret-ltr-right-expected.txt: Added.
     38        * platform/mac/editing/selection/caret-rtl-2-expected.checksum:
     39        * platform/mac/editing/selection/caret-rtl-2-expected.png:
     40        * platform/mac/editing/selection/caret-rtl-2-expected.txt:
     41        * platform/mac/editing/selection/caret-rtl-2-left-expected.checksum: Added.
     42        * platform/mac/editing/selection/caret-rtl-2-left-expected.png: Added.
     43        * platform/mac/editing/selection/caret-rtl-2-left-expected.txt: Added.
     44        * platform/mac/editing/selection/caret-rtl-expected.checksum:
     45        * platform/mac/editing/selection/caret-rtl-expected.png:
     46        * platform/mac/editing/selection/caret-rtl-expected.txt:
     47        * platform/mac/editing/selection/caret-rtl-right-expected.checksum: Added.
     48        * platform/mac/editing/selection/caret-rtl-right-expected.png: Added.
     49        * platform/mac/editing/selection/caret-rtl-right-expected.txt: Added.
     50
    1512010-12-08  Alexey Proskuryakov  <ap@apple.com>
    252

  • trunk/LayoutTests/editing/selection/caret-rtl-2.html

    r17562 r73548  
     1<!DOCTYPE html>
    12<html>
    2 <head>
     3<body>
     4<p>
     5This tests that clicking in a contenteditable div will set the caret in the right position for RTL text in a RTL block.
     6To test manually, click the right of the text. The caret should be on the right edge.
     7</p>
     8<div style="font-size: 20px; width: 20ex; border: solid thin black; padding: 10px;" contenteditable>&#x05e9;&#x05d3;&#x05d4; &#x05d1;&#x05d5;&#x05e8;</div>
     9<script src="resources/caret-edge-shared.js"></script>
    310<script>
    4 if (window.layoutTestController)
    5      layoutTestController.dumpEditingCallbacks();
     11
     12var clickOn = 'right';
     13var expectedOffset = 7;
     14
     15runTest();
     16
    617</script>
    7 
    8 <script>
    9 function test()
    10 {   
    11     if (window.layoutTestController) {
    12         eventSender.mouseMoveTo(145, 75);
    13         eventSender.mouseDown();
    14         eventSender.mouseUp();
    15     }
    16 }
    17 </script>
    18 </head>
    19 <body onload="test()">
    20 This tests that clicking in a contenteditable div will set the caret in the right position for rtl text.
    21 <br>
    22 <p>
    23     Click in the middle of the field.  The caret should be on the edge of the text closer to where you clicked.
    24 </p>
    25 <div style="width:150px; border: solid thin black; position: absolute; top: 70; left: 10;" contenteditable>&#x05e9;&#x05d3;&#x05d4; &#x05d1;&#x05d5;&#x05e8;</div>
    2618</body>
    2719</html>
    28 

  • trunk/LayoutTests/editing/selection/caret-rtl.html

    r17562 r73548  
     1<!DOCTYPE html>
    12<html>
    2 <head>
     3<body>
     4<p>
     5This tests that clicking in a contenteditable div will set the caret in the right position for RTL text in a RTL block.
     6To test manually, click the left of the text. The caret should be on the left edge.
     7</p>
     8<div style="direction: rtl; font-size: 20px; width: 20ex; border: solid thin black; padding: 10px;" contenteditable>&#x05e9;&#x05d3;&#x05d4; &#x05d1;&#x05d5;&#x05e8;</div>
     9<script src="resources/caret-edge-shared.js"></script>
    310<script>
    4 if (window.layoutTestController)
    5      layoutTestController.dumpEditingCallbacks();
     11
     12var clickOn = 'left';
     13var expectedOffset = 7;
     14
     15runTest();
     16
    617</script>
    7 
    8 <script>
    9 function test()
    10 {   
    11     if (window.layoutTestController) {
    12         eventSender.mouseMoveTo(25, 75);
    13         eventSender.mouseDown();
    14         eventSender.mouseUp();
    15     }
    16 }
    17 </script>
    18 </head>
    19 <body onload="test()">
    20 This tests that clicking in a contenteditable div will set the caret in the right position for rtl text.
    21 <br>
    22 <p>
    23     Click in the middle of the field.  The caret should be on the edge of the text closer to where you clicked.
    24 </p>
    25 <div style="direction: rtl; width:150px; border: solid thin black; position: absolute; top: 70; left: 10;" contenteditable>&#x05e9;&#x05d3;&#x05d4; &#x05d1;&#x05d5;&#x05e8;</div>
    2618</body>
    2719</html>
    28 

  • trunk/LayoutTests/platform/mac/editing/selection/caret-rtl-2-expected.checksum

    r72177 r73548  
    1 7fe6fd218948722ede6f6937ad29c6e3
     18c48e5709f8fded01efd36c9590039de

  • trunk/LayoutTests/platform/mac/editing/selection/caret-rtl-2-expected.txt

    r72177 r73548  
    1 EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
    2 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
    3 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    4 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    51layer at (0,0) size 800x600
    62  RenderView at (0,0) size 800x600
    7 layer at (0,0) size 800x600
    8   RenderBlock {HTML} at (0,0) size 800x600
    9     RenderBody {BODY} at (8,8) size 784x576
    10       RenderBlock (anonymous) at (0,0) size 784x18
    11         RenderText {#text} at (0,0) size 596x18
    12           text run at (0,0) width 596: "This tests that clicking in a contenteditable div will set the caret in the right position for rtl text. "
    13         RenderBR {BR} at (596,14) size 0x0
    14       RenderBlock {P} at (0,34) size 784x18
    15         RenderText {#text} at (0,0) size 649x18
    16           text run at (0,0) width 200: "Click in the middle of the field. "
    17           text run at (200,0) width 449: "The caret should be on the edge of the text closer to where you clicked."
    18 layer at (10,70) size 152x20
    19   RenderBlock (positioned) {DIV} at (10,70) size 152x20 [border: (1px solid #000000)]
    20     RenderText {#text} at (1,1) size 58x18
    21       text run at (1,1) width 58 RTL: "\x{5E9}\x{5D3}\x{5D4} \x{5D1}\x{5D5}\x{5E8}"
    22 caret: position 0 of child 0 {#text} of child 5 {DIV} of body
     3layer at (0,0) size 800x139
     4  RenderBlock {HTML} at (0,0) size 800x139
     5    RenderBody {BODY} at (8,16) size 784x115
     6      RenderBlock {P} at (0,0) size 784x36
     7        RenderText {#text} at (0,0) size 758x36
     8          text run at (0,0) width 758: "This tests that clicking in a contenteditable div will set the caret in the right position for RTL text in a RTL block. To test"
     9          text run at (0,18) width 465: "manually, click the right of the text. The caret should be on the right edge."
     10      RenderBlock {DIV} at (0,52) size 213x45 [border: (1px solid #000000)]
     11        RenderText {#text} at (11,11) size 72x23
     12          text run at (11,11) width 72 RTL: "\x{5E9}\x{5D3}\x{5D4} \x{5D1}\x{5D5}\x{5E8}"
     13      RenderBlock (anonymous) at (0,97) size 784x18
     14        RenderText {#text} at (0,0) size 39x18
     15          text run at (0,0) width 39: "PASS"
     16        RenderBR {BR} at (39,0) size 0x18
     17caret: position 7 of child 0 {#text} of child 3 {DIV} of body

  • trunk/LayoutTests/platform/mac/editing/selection/caret-rtl-expected.checksum

    r60107 r73548  
    1 6a84f0df35ac5f65f199cba48dcd50bc
     19b45da74056c05d3be1203a87812ef3b

  • trunk/LayoutTests/platform/mac/editing/selection/caret-rtl-expected.txt

    r72179 r73548  
    1 EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
    2 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
    3 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 7 of #text > DIV > BODY > HTML > #document to 7 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    4 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    51layer at (0,0) size 800x600
    62  RenderView at (0,0) size 800x600
    7 layer at (0,0) size 800x600
    8   RenderBlock {HTML} at (0,0) size 800x600
    9     RenderBody {BODY} at (8,8) size 784x576
    10       RenderBlock (anonymous) at (0,0) size 784x18
    11         RenderText {#text} at (0,0) size 596x18
    12           text run at (0,0) width 596: "This tests that clicking in a contenteditable div will set the caret in the right position for rtl text. "
    13         RenderBR {BR} at (596,14) size 0x0
    14       RenderBlock {P} at (0,34) size 784x18
    15         RenderText {#text} at (0,0) size 649x18
    16           text run at (0,0) width 200: "Click in the middle of the field. "
    17           text run at (200,0) width 449: "The caret should be on the edge of the text closer to where you clicked."
    18 layer at (10,70) size 152x20
    19   RenderBlock (positioned) {DIV} at (10,70) size 152x20 [border: (1px solid #000000)]
    20     RenderText {#text} at (93,1) size 58x18
    21       text run at (93,1) width 58 RTL: "\x{5E9}\x{5D3}\x{5D4} \x{5D1}\x{5D5}\x{5E8}"
    22 caret: position 7 of child 0 {#text} of child 5 {DIV} of body
     3layer at (0,0) size 800x139
     4  RenderBlock {HTML} at (0,0) size 800x139
     5    RenderBody {BODY} at (8,16) size 784x115
     6      RenderBlock {P} at (0,0) size 784x36
     7        RenderText {#text} at (0,0) size 758x36
     8          text run at (0,0) width 758: "This tests that clicking in a contenteditable div will set the caret in the right position for RTL text in a RTL block. To test"
     9          text run at (0,18) width 447: "manually, click the left of the text. The caret should be on the left edge."
     10      RenderBlock {DIV} at (0,52) size 213x45 [border: (1px solid #000000)]
     11        RenderText {#text} at (130,11) size 72x23
     12          text run at (130,11) width 72 RTL: "\x{5E9}\x{5D3}\x{5D4} \x{5D1}\x{5D5}\x{5E8}"
     13      RenderBlock (anonymous) at (0,97) size 784x18
     14        RenderText {#text} at (0,0) size 39x18
     15          text run at (0,0) width 39: "PASS"
     16        RenderBR {BR} at (39,0) size 0x18
     17caret: position 7 of child 0 {#text} of child 3 {DIV} of body

  • trunk/WebCore/ChangeLog

    r73543 r73548  
     12010-12-08  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Dan Bernstein.
     4
     5        REGRESSION: LayoutTests/editing/selection/caret-rtl-2.html fails
     6        https://bugs.webkit.org/show_bug.cgi?id=33503
     7
     8        The bug was caused by Font::offsetForPosition's not taking into account the containing block's text direction.
     9        When RTL text appears in a LTR block, the offset at the beginning of RTL text is on the left of RTL text,
     10        adn the offset at the end of RTL text is on the right of RTL text. For example, if we had RTL text CBA,
     11        then the correspondance between letters and offsets in logical order are: A -> 0, B -> 1, and C -> 2.
     12
     13        Case 1. CBA appears in a RTL block:
     14          In this case, clicking on the visual left of CBA puts the caret naturally at Position("CBA", 2).
     15          Clicking on the visual right of CBA puts the caret at Position("CBA", 0) as expected.
     16        Case 2. CBA appears in a LTR block:
     17          Because the containing block flows from left to right, by covention, Position("CBA", 2") coresponds
     18          to the visual right of CBA, and Position("CBA", 0) corresponds to the visual left of CBA.
     19          Therefore, clicking on the visual left of CBA should put the caret at Positoin("CBA", 0),
     20          and clicking on the visual right should put it at Position("CBA", 2).
     21
     22        The bug was caused by WebKit's not considering case 2. The same bug also existe for LTR text in a RTL block.
     23        Fixed the bug by taking care of the case 2 in InlineTextBox::offsetForPosition.
     24
     25        Tests: editing/selection/caret-ltr-2-left.html
     26               editing/selection/caret-ltr-2.html
     27               editing/selection/caret-ltr-right.html
     28               editing/selection/caret-ltr.html
     29               editing/selection/caret-rtl-2-left.html
     30               editing/selection/caret-rtl-right.html
     31
     32        * rendering/InlineTextBox.cpp:
     33        (WebCore::InlineTextBox::offsetForPosition):
     34
    1352010-12-08  Anders Carlsson  <andersca@apple.com>
    236

  • trunk/WebCore/rendering/InlineTextBox.cpp

    r73284 r73548  
    10981098        return 0;
    10991099
     1100    int leftOffset = isLeftToRightDirection() ? 0 : m_len;
     1101    int rightOffset = isLeftToRightDirection() ? m_len : 0;
     1102    if (renderer()->containingBlock()->style()->isLeftToRightDirection() != isLeftToRightDirection())
     1103        swap(leftOffset, rightOffset);
     1104
     1105    if (lineOffset - logicalLeft() > logicalWidth())
     1106        return rightOffset;
     1107    if (lineOffset - logicalLeft() < 0)
     1108        return leftOffset;
     1109
    11001110    RenderText* text = toRenderText(renderer());
    11011111    RenderStyle* style = text->style(m_firstLine);
    11021112    const Font* f = &style->font();
    11031113    return f->offsetForPosition(TextRun(textRenderer()->text()->characters() + m_start, m_len, textRenderer()->allowTabs(), textPos(), m_toAdd, !isLeftToRightDirection(), m_dirOverride || style->visuallyOrdered()),
    1104                                 lineOffset - logicalLeft(), includePartialGlyphs);
     1114        lineOffset - logicalLeft(), includePartialGlyphs);
    11051115}
    11061116
Note: See TracChangeset for help on using the changeset viewer.