Bug 118786 – Correct previous line position not returned when moving the caret between lines containing non-editable nodes.

• Component: HTML Editing
• Status: NEW

[Arpita Bahuguna]

Created attachment 206875 [details]
Testpage

Steps to reproduce the issue:
1. Fetch the attached testpage and place the caret at the end of the third line "2nd line here".
2. Press the "up" arrow key for moving the caret one line up.
3. Notice that the caret moves directly to the first line skipping the in-between line containing the image.

The previous line position computation is not correct since it skips the position corresponding to the non-editable node and instead moves to the line before that.

[Arpita Bahuguna]

Created attachment 206881 [details]
Patch

[Arpita Bahuguna]

The patch will make our behavior similar to that of Firefox.

[Build Bot]

Comment on attachment 206881 [details]
Patch

Attachment 206881 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1101198

New failing tests:
editing/selection/3690703-2.html

[Build Bot]

Created attachment 206895 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 206881 [details]
Patch

Attachment 206881 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1090695

New failing tests:
editing/selection/3690703-2.html

[Build Bot]

Created attachment 206941 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Arpita Bahuguna]

Hi Ryosuke,
Would really like your help with this issue.

If the computed point on the previous line corresponds to a editing ignored element, we move the caret to a position "before" such a node.

    if (root) {
        IntPoint pointInLine = absoluteLineDirectionPointToLocalPointInBlock(root, lineDirectionPoint);

        RenderObject* renderer = root->closestLeafChildForPoint(pointInLine, isEditablePosition(p))->renderer();
        Node* node = renderer->node();

        if (node && editingIgnoresContent(node))
            return positionInParentBeforeNode(node);
        return renderer->positionForPoint(pointInLine);
    }

However, for the attached testcase (3 lines with second containing an image), when moving up from the end of the last line, the caret should rest at the end or "after" image node. But since we call positionInParentBeforeNode(), the before node position for image element is returned which is nothing but the end of the previous text line. So currently the caret moves to the end of the first line which is incorrect behavior. Both FF and IE handle it properly.

But if I were to change the default call to positionInParentAfterNode(), cases like the regression for the given fix, i.e editing/selection/3690703-2.html, shall fail. Even though in my opinion when moving to the previous line we should be seeking for the position after node and when moving to the next line we should be seeking for the position before node.

Another thing is that if our node corresponds to a BR element or other text control elements, in that case too our caret should always be moved to the position before node.

I tried adding a specific check like:
    if ((node->isElementNode() && toElement(node)->isFormControlElement()) || renderer->isBR())
        return positionInParentBeforeNode(node);
    else
        return positionInParentAfterNode(node);

which seems to work for all the existing testcases and the attached issue case here, but if say our previous line contains two or more inline images, the caret then goes and rests and the end of those two images instead of "before" the last image, as is the current behavior.

I would really appreciate your thoughts on this and on how to move forward with this issue.

[Arpita Bahuguna]

Also, in continuation with the previous comment, Firefox seems to always move the caret to the after node position when moving to the previous line.

[Arpita Bahuguna]

Created attachment 207390 [details]
Detailed testcase

Detailed testcase for verifying up/down caret movement between lines containing non-editable elements.

[Arpita Bahuguna]

Created attachment 207393 [details]
Patch

[Arpita Bahuguna]

Hi rniwa, have added another patch based on the premise that when moving up we should seek a position that lies after a non-editable node. Only in case the element is either a <br> or a form control element should we go for a position that lies before such nodes.
This is as per my comment #7.

[Arpita Bahuguna]

Comment on attachment 207393 [details]
Patch

Submitted patch for this issue along with https://bugs.webkit.org/show_bug.cgi?id=106827.