Changeset 41191 in webkit


Ignore:
Timestamp:
Feb 24, 2009 2:28:31 PM (15 years ago)
Author:
eric@webkit.org
Message:

2009-02-05 Ojan Vafai <​ojan@chromium.org> and Eric Seidel <​eric@webkit.org>

Reviewed by Dave Hyatt.

Make cursor positions match IE6/IE7/FF3 when clicking in margins/padding
around divs inside editable regions.
​https://bugs.webkit.org/show_bug.cgi?id=23605

Fix clicks outside editable regions from focusing the editable region.
​https://bugs.webkit.org/show_bug.cgi?id=23607

Removed editing/selection/contenteditable-click-outside.html as it's
not as useful as our new tests.

Clean up RenderBlock::positionForCoordinates to remove dead code,
duplicate code, and generally make it more readable.

Tests: editing/selection/click-in-margins-inside-editable-div.html

editing/selection/click-in-padding-with-multiple-line-boxes.html
editing/selection/click-outside-editable-div.html

  • editing/VisiblePosition.cpp: (WebCore::VisiblePosition::canonicalPosition):
  • rendering/RenderBlock.cpp: (WebCore::positionForPointRespectingEditingBoundaries): (WebCore::positionForPointWithInlineChildren): (WebCore::RenderBlock::positionForCoordinates): (WebCore::RenderBlock::updateFirstLetter):
Location:
trunk
Files:
12 added
5 deleted
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r41189 r41191  
     12009-02-05 Ojan Vafai <ojan@chromium.org> and Eric Seidel <eric@webkit.org>
     2
     3        Reviewed by Dave Hyatt.
     4
     5        Make cursor positions match IE6/IE7/FF3 when clicking in margins/padding
     6        around divs inside editable regions.
     7        https://bugs.webkit.org/show_bug.cgi?id=23605
     8
     9        Fix clicks outside editable regions from focusing the editable region.
     10        https://bugs.webkit.org/show_bug.cgi?id=23607
     11
     12        Removed editing/selection/contenteditable-click-outside.html as it's
     13        not as useful as our new tests.
     14
     15        * editing/selection/click-in-margins-inside-editable-div-expected.txt: Added.
     16        * editing/selection/click-in-margins-inside-editable-div.html: Added.
     17        * editing/selection/click-in-padding-with-multiple-line-boxes-expected.txt: Added.
     18        * editing/selection/click-in-padding-with-multiple-line-boxes.html: Added.
     19        * editing/selection/click-outside-editable-div-expected.txt: Added.
     20        * editing/selection/click-outside-editable-div.html: Added.
     21        * editing/selection/contenteditable-click-outside.html: Removed.
     22        * editing/selection/resources/TEMPLATE.html: Added.
     23        * editing/selection/resources/click-in-margins-inside-editable-div.js: Added.
     24        * editing/selection/resources/click-in-padding-with-multiple-line-boxes.js: Added.
     25        * editing/selection/resources/click-outside-editable-div.js: Added.
     26        * editing/selection/resources/js-test-selection-shared.js: Added.
     27        * editing/selection/select-missing-image.html:
     28        * platform/mac/editing/selection/contenteditable-click-outside-expected.checksum: Removed.
     29        * platform/mac/editing/selection/contenteditable-click-outside-expected.txt: Removed.
     30        * platform/mac/editing/selection/select-all-iframe-expected.txt:
     31        * platform/mac/editing/selection/select-from-textfield-outwards-expected.txt:
     32        * platform/mac/editing/selection/select-missing-image-expected.txt:
     33        * platform/qt/editing/selection/contenteditable-click-outside-expected.txt: Removed.
     34        * platform/qt/editing/selection/select-all-iframe-expected.txt: Removed.
     35
    1362009-02-24  Simon Fraser  <simon.fraser@apple.com>
    237

  • trunk/LayoutTests/editing/selection/select-missing-image.html

    r17912 r41191  
    2222</head>
    2323<body>
    24 <div contenteditable id="root" class="editing">
     24<div contenteditable id="test" class="editing">
    2525<img title="Should see selection tint over this missing image." src="../noneSuch.jpg" height="100" width="550">
    2626</div>

  • trunk/LayoutTests/platform/mac/editing/selection/select-all-iframe-expected.txt

    r30635 r41191  
    22EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of BODY > HTML > #document to 4 of BODY > HTML > #document
    33EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
    4 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 11 of #text > DIV > BODY > HTML > #document to 11 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     4EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 5 of #text > DIV > BODY > HTML > #document to 5 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    55EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    6 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 11 of #text > DIV > BODY > HTML > #document to 11 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 11 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
     6EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > DIV > BODY > HTML > #document to 5 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 11 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    77EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    88EDITING DELEGATE: shouldDeleteDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 11 of #text > DIV > BODY > HTML > #document

  • trunk/LayoutTests/platform/mac/editing/selection/select-from-textfield-outwards-expected.txt

    r30635 r41191  
    99EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 11 of #text > DIV to 17 of #text > DIV toDOMRange:range from 0 of #text > DIV to 12 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1010EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    11 EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV to 12 of #text > DIV toDOMRange:range from 0 of #text > DIV to 12 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1211EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of #text > DIV to 12 of #text > DIV toDOMRange:range from 11 of #text > DIV to 17 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1312EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
     13EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 11 of #text > DIV to 17 of #text > DIV toDOMRange:range from 11 of #text > DIV to 17 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1414EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 11 of #text > DIV to 17 of #text > DIV toDOMRange:range from 11 of #text > DIV to 17 of #text > DIV affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    1515layer at (0,0) size 800x600

  • trunk/LayoutTests/platform/mac/editing/selection/select-missing-image-expected.txt

    r30635 r41191  
    1 EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of DIV > BODY > HTML > #document to 2 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    21EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 3 of DIV > BODY > HTML > #document
    32EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
     3EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
     4EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 1 of DIV > BODY > HTML > #document to 2 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
    45EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
    56layer at (0,0) size 800x600

  • trunk/WebCore/ChangeLog

    r41190 r41191  
     12009-02-05  Ojan Vafai  <ojan@chromium.org> and Eric Seidel <eric@webkit.org>
     2
     3        Reviewed by Dave Hyatt.
     4
     5        Make cursor positions match IE6/IE7/FF3 when clicking in margins/padding
     6        around divs inside editable regions.
     7        https://bugs.webkit.org/show_bug.cgi?id=23605
     8
     9        Fix clicks outside editable regions from focusing the editable region.
     10        https://bugs.webkit.org/show_bug.cgi?id=23607
     11
     12        Removed editing/selection/contenteditable-click-outside.html as it's
     13        not as useful as our new tests.
     14
     15        Clean up RenderBlock::positionForCoordinates to remove dead code,
     16        duplicate code, and generally make it more readable.
     17
     18        Tests: editing/selection/click-in-margins-inside-editable-div.html
     19               editing/selection/click-in-padding-with-multiple-line-boxes.html
     20               editing/selection/click-outside-editable-div.html
     21
     22        * editing/VisiblePosition.cpp:
     23        (WebCore::VisiblePosition::canonicalPosition):
     24        * rendering/RenderBlock.cpp:
     25        (WebCore::positionForPointRespectingEditingBoundaries):
     26        (WebCore::positionForPointWithInlineChildren):
     27        (WebCore::RenderBlock::positionForCoordinates):
     28        (WebCore::RenderBlock::updateFirstLetter):
     29
    1302009-02-24  Sam Weinig  <sam@webkit.org>
    231

  • trunk/WebCore/editing/VisiblePosition.cpp

    r40871 r41191  
    477477    Node* prevNode = prev.node();
    478478
     479    // If the body is null, there is no visible position.
     480    if (!node->ownerDocument()->body())
     481        return Position();
     482
    479483    // The new position must be in the same editable element. Enforce that first.
    480484    // Unless the descent is from a non-editable html element to an editable body.
    481     if (node->hasTagName(htmlTag) && !node->isContentEditable())
     485    if (node->hasTagName(htmlTag) && !node->isContentEditable() && node->ownerDocument()->body()->isContentEditable())
    482486        return next.isNotNull() ? next : prev;
    483487
    … …  
    493497    if (prevIsInSameEditableElement && !nextIsInSameEditableElement)
    494498        return prev;
    495        
     499
    496500    if (nextIsInSameEditableElement && !prevIsInSameEditableElement)
    497501        return next;
    498        
     502
    499503    if (!nextIsInSameEditableElement && !prevIsInSameEditableElement)
    500504        return Position();

  • trunk/WebCore/rendering/RenderBlock.cpp

    r41153 r41191  
    34173417}
    34183418
     3419// FIXME: This function should go on RenderObject as an instance method. Then
     3420// all cases in which positionForCoordinates recurs could call this instead to
     3421// prevent crossing editable boundaries. This would require many tests.
     3422static VisiblePosition positionForPointRespectingEditingBoundaries(RenderBox* parent, RenderBox* child, const IntPoint& parentCoords)
     3423{
     3424    int xInChildCoords = parentCoords.x() - child->x();
     3425    int yInChildCoords = parentCoords.y() - child->y();
     3426
     3427    // If this is an anonymous renderer, we just recur normally
     3428    Node* childNode = child->node();
     3429    if (!childNode)
     3430        return child->positionForCoordinates(xInChildCoords, yInChildCoords);
     3431
     3432    // Otherwise, first make sure that the editability of the parent and child agree.
     3433    // If they don't agree, then we return a visible position just before or after the child
     3434    RenderObject* ancestor = parent;
     3435    while (ancestor && !ancestor->node())
     3436        ancestor = ancestor->parent();
     3437
     3438    // If we can't find an ancestor to check editability on, or editability is unchanged, we recur like normal
     3439    if (!ancestor || ancestor->node()->isContentEditable() == childNode->isContentEditable())
     3440        return child->positionForCoordinates(xInChildCoords, yInChildCoords);
     3441
     3442    // Otherwise return before or after the child, depending on if the click was left or right of the child
     3443    int childMidX = child->width() / 2;
     3444    if (xInChildCoords < childMidX)
     3445        return VisiblePosition(ancestor->node(), childNode->nodeIndex(), DOWNSTREAM);
     3446    return VisiblePosition(ancestor->node(), childNode->nodeIndex() + 1, UPSTREAM);
     3447}
     3448
     3449static VisiblePosition positionForPointWithInlineChildren(RenderBlock* block, const IntPoint& pointInContents)
     3450{
     3451    ASSERT(block->childrenInline());
     3452
     3453    if (!block->firstRootBox())
     3454        return VisiblePosition(block->node(), 0, DOWNSTREAM);
     3455
     3456    InlineBox* closestBox = 0;
     3457    // look for the closest line box in the root box which is at the passed-in y coordinate
     3458    for (RootInlineBox* root = block->firstRootBox(); root; root = root->nextRootBox()) {
     3459        int bottom;
     3460        // set the bottom based on whether there is a next root box
     3461        if (root->nextRootBox())
     3462            // FIXME: make the break point halfway between the bottom of the previous root box and the top of the next root box
     3463            bottom = root->nextRootBox()->topOverflow();
     3464        else
     3465            bottom = root->bottomOverflow() + verticalLineClickFudgeFactor;
     3466        // check if this root line box is located at this y coordinate
     3467        if (pointInContents.y() < bottom && root->firstChild()) {
     3468            closestBox = root->closestLeafChildForXPos(pointInContents.x());
     3469            if (closestBox)
     3470                // pass the box a y position that is inside it
     3471                break;
     3472        }
     3473    }
     3474
     3475    // y coordinate is below last root line box, pretend we hit it
     3476    if (!closestBox)
     3477        closestBox = block->lastRootBox()->closestLeafChildForXPos(pointInContents.x());
     3478
     3479    if (closestBox) {
     3480        // pass the box a y position that is inside it
     3481        return closestBox->renderer()->positionForCoordinates(pointInContents.x(), closestBox->m_y);
     3482    }
     3483
     3484    // Can't reach this.  We have a root line box, but it has no kids.
     3485    ASSERT_NOT_REACHED();
     3486    return VisiblePosition(block->node(), 0, DOWNSTREAM);
     3487}
     3488
    34193489VisiblePosition RenderBlock::positionForCoordinates(int x, int y)
    34203490{
    … …  
    34223492        return RenderBox::positionForCoordinates(x, y);
    34233493
    3424     int top = borderTop();
    3425     int bottom = top + paddingTop() + contentHeight() + paddingBottom();
    3426 
    3427     int left = borderLeft();
    3428     int right = left + paddingLeft() + contentWidth() + paddingRight();
    3429 
    3430     Node* n = node();
    3431    
    34323494    int contentsX = x;
    34333495    int contentsY = y;
    34343496    offsetForContents(contentsX, contentsY);
     3497    IntPoint pointInContents(contentsX, contentsY);
    34353498
    34363499    if (isReplaced()) {
    34373500        if (y < 0 || y < height() && x < 0)
    3438             return VisiblePosition(n, caretMinOffset(), DOWNSTREAM);
     3501            return VisiblePosition(node(), caretMinOffset(), DOWNSTREAM);
    34393502        if (y >= height() || y >= 0 && x >= width())
    3440             return VisiblePosition(n, caretMaxOffset(), DOWNSTREAM);
     3503            return VisiblePosition(node(), caretMaxOffset(), DOWNSTREAM);
    34413504    }
    34423505
    3443     // If we start inside the shadow tree, we will stay inside (even if the point is above or below).
    3444     if (!(n && n->isShadowNode()) && !childrenInline()) {
    3445         // Don't return positions inside editable roots for coordinates outside those roots, except for coordinates outside
    3446         // a document that is entirely editable.
    3447         bool isEditableRoot = n && n->rootEditableElement() == n && !n->hasTagName(bodyTag) && !n->hasTagName(htmlTag);
    3448 
    3449         if (y < top || (isEditableRoot && (y < bottom && x < left))) {
    3450             if (!isEditableRoot)
    3451                 if (RenderBox* c = firstChildBox()) { // FIXME: This code doesn't make any sense.  This child could be an inline or a positioned element or a float, etc.
    3452                     VisiblePosition p = c->positionForCoordinates(contentsX - c->x(), contentsY - c->y());
    3453                     if (p.isNotNull())
    3454                         return p;
    3455                 }
    3456             if (n) {
    3457                 if (Node* sp = n->shadowParentNode())
    3458                     n = sp;
    3459                 if (Node* p = n->parent())
    3460                     return VisiblePosition(p, n->nodeIndex(), DOWNSTREAM);
    3461             }
    3462             return VisiblePosition(n, 0, DOWNSTREAM);
    3463         }
    3464 
    3465         if (y >= bottom || (isEditableRoot && (y >= top && x >= right))) {
    3466             if (!isEditableRoot)
    3467                 if (RenderBox* c = lastChildBox()) { // FIXME: This code doesn't make any sense.  This child could be an inline or a positioned element or a float, etc.
    3468                     VisiblePosition p = c->positionForCoordinates(contentsX - c->x(), contentsY - c->y());
    3469                     if (p.isNotNull())
    3470                         return p;
    3471                 }
    3472             if (n) {
    3473                 if (Node* sp = n->shadowParentNode())
    3474                     n = sp;
    3475                 if (Node* p = n->parent())
    3476                     return VisiblePosition(p, n->nodeIndex() + 1, DOWNSTREAM);
    3477             }
    3478             return VisiblePosition(n, 0, DOWNSTREAM);
    3479         }
    3480     }
    3481 
    34823506    if (childrenInline()) {
    3483         if (!firstRootBox())
    3484             return VisiblePosition(n, 0, DOWNSTREAM);
    3485 
    3486         if (contentsY < firstRootBox()->topOverflow() - verticalLineClickFudgeFactor)
    3487             // y coordinate is above first root line box
    3488             return VisiblePosition(positionForBox(firstRootBox()->firstLeafChild(), true), DOWNSTREAM);
    3489        
    3490         // look for the closest line box in the root box which is at the passed-in y coordinate
    3491         for (RootInlineBox* root = firstRootBox(); root; root = root->nextRootBox()) {
    3492             // set the bottom based on whether there is a next root box
    3493             if (root->nextRootBox())
    3494                 // FIXME: make the break point halfway between the bottom of the previous root box and the top of the next root box
    3495                 bottom = root->nextRootBox()->topOverflow();
    3496             else
    3497                 bottom = root->bottomOverflow() + verticalLineClickFudgeFactor;
    3498             // check if this root line box is located at this y coordinate
    3499             if (contentsY < bottom && root->firstChild()) {
    3500                 InlineBox* closestBox = root->closestLeafChildForXPos(x);
    3501                 if (closestBox)
    3502                     // pass the box a y position that is inside it
    3503                     return closestBox->renderer()->positionForCoordinates(contentsX, closestBox->m_y);
    3504             }
    3505         }
    3506 
    3507         if (lastRootBox())
    3508             // y coordinate is below last root line box
    3509             return VisiblePosition(positionForBox(lastRootBox()->lastLeafChild(), false), DOWNSTREAM);
    3510 
    3511         return VisiblePosition(n, 0, DOWNSTREAM);
    3512     }
    3513    
    3514     // See if any child blocks exist at this y coordinate.
     3507        return positionForPointWithInlineChildren(this, pointInContents);
     3508    }
     3509
     3510    // Check top/bottom child-margin/parent-padding for clicks and place them in the first/last child
     3511    // FIXME: This will not correctly handle first or last children being positioned or non-visible
    35153512    if (firstChildBox() && contentsY < firstChildBox()->y())
    3516         return VisiblePosition(n, 0, DOWNSTREAM);
    3517     for (RenderBox* renderer = firstChildBox(); renderer; renderer = renderer->nextSiblingBox()) {
    3518         if (renderer->height() == 0 || renderer->style()->visibility() != VISIBLE || renderer->isFloatingOrPositioned())
     3513        return positionForPointRespectingEditingBoundaries(this, firstChildBox(), pointInContents);
     3514    if (lastChildBox() && contentsY > lastChildBox()->y())
     3515        return positionForPointRespectingEditingBoundaries(this, lastChildBox(), pointInContents);
     3516
     3517    for (RenderBox* childBox = firstChildBox(); childBox; childBox = childBox->nextSiblingBox()) {
     3518        if (childBox->height() == 0 || childBox->style()->visibility() != VISIBLE || childBox->isFloatingOrPositioned())
    35193519            continue;
    3520         RenderBox* next = renderer->nextSiblingBox();
    3521         while (next && next->isFloatingOrPositioned())
    3522             next = next->nextSiblingBox();
    3523         if (next)
    3524             bottom = next->y();
    3525         else
    3526             bottom = top + scrollHeight();
    3527         if (contentsY >= renderer->y() && contentsY < bottom)
    3528             return renderer->positionForCoordinates(contentsX - renderer->x(), contentsY - renderer->y());
    3529     }
    3530    
     3520        // We hit this child if our click was above the bottom of its padding box (like IE6/7 and FF3)
     3521        if (contentsY < childBox->y() + childBox->height())
     3522            return positionForPointRespectingEditingBoundaries(this, childBox, pointInContents);
     3523    }
     3524
     3525    // We only get here if there are no, or only floated/positioned, or only
     3526    // non-visible block children below the click.
    35313527    return RenderBox::positionForCoordinates(x, y);
    35323528}
    … …  
    44864482    if (!document()->usesFirstLetterRules())
    44874483        return;
    4488     // Don't recurse
     4484    // Don't recur
    44894485    if (style()->styleType() == FIRST_LETTER)
    44904486        return;
Note: See TracChangeset for help on using the changeset viewer.