WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 23607
REGRESSION: Clicking near an contentEditable div will focus the div, even though it shouldn't!
https://bugs.webkit.org/show_bug.cgi?id=23607
Summary
REGRESSION: Clicking near an contentEditable div will focus the div, even tho...
Eric Seidel (no email)
Reported
2009-01-28 16:15:13 PST
Clicking near an contentEditable div will focus the div, even though it shouldn't! This is especially noticeable if the div is the only content in the page (as any click on the body will focus the div). FF and IE get this right. This blocks conversion of text areas away from shadow nodes.
Attachments
js portion of test case
(1.41 KB, application/x-javascript)
2009-01-28 16:16 PST
,
Eric Seidel (no email)
no flags
Details
First pass testcases for bug 23605 and bug 23607
(9.83 KB, patch)
2009-01-30 14:50 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
First pass fix, Position::cannonicalPosition still seems to break us
(9.23 KB, patch)
2009-01-30 14:50 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Fix bugs 23605 and 23607
(37.54 KB, patch)
2009-01-30 17:54 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Fix bugs 23605 and 23607
(37.58 KB, patch)
2009-01-30 18:26 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Fix bugs 23605 and 23607
(37.58 KB, patch)
2009-01-30 22:12 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Fix bugs 23605 and 23607
(39.88 KB, patch)
2009-01-30 22:14 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
A full git patch for easier applying by ojan (with git am)
(57.28 KB, patch)
2009-02-02 17:58 PST
,
Eric Seidel (no email)
no flags
Details
Formatted Diff
Diff
Fix positionForCoordinates now with an added fix for clicking in the margins of the body.
(41.54 KB, patch)
2009-02-05 17:42 PST
,
Ojan Vafai
hyatt
: review-
Details
Formatted Diff
Diff
Fix for clicking below the last root line box with multiple line boxes.
(45.83 KB, patch)
2009-02-12 16:02 PST
,
Ojan Vafai
hyatt
: review+
Details
Formatted Diff
Diff
Same patch, but synced to trunk for easier landing by eseidel.
(45.72 KB, patch)
2009-02-24 13:25 PST
,
Ojan Vafai
no flags
Details
Formatted Diff
Diff
patch made with git format-patch for even easier patching by eseidel. :)
(48.99 KB, text/plain)
2009-02-24 13:32 PST
,
Ojan Vafai
no flags
Details
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2009-01-28 16:16:01 PST
Created
attachment 27133
[details]
js portion of test case
Eric Seidel (no email)
Comment 2
2009-01-30 14:50:08 PST
Created
attachment 27196
[details]
First pass testcases for
bug 23605
and
bug 23607
...ick-in-margins-inside-editable-div-expected.txt | 9 ++ .../click-in-margins-inside-editable-div.html | 13 ++ .../click-outside-editable-div-expected.txt | 15 ++ .../selection/click-outside-editable-div.html | 13 ++ .../editing/selection/resources/TEMPLATE.html | 13 ++ .../click-in-margins-inside-editable-div.js | 135 ++++++++++++++++++++ .../resources/click-outside-editable-div.js | 50 +++++++ 7 files changed, 248 insertions(+), 0 deletions(-)
Eric Seidel (no email)
Comment 3
2009-01-30 14:50:09 PST
Created
attachment 27197
[details]
First pass fix, Position::cannonicalPosition still seems to break us WebCore/rendering/RenderBlock.cpp | 112 +++++++++++++++------------------ WebCore/rendering/RenderContainer.cpp | 3 +- WebCore/rendering/RenderInline.cpp | 1 + 3 files changed, 54 insertions(+), 62 deletions(-)
Eric Seidel (no email)
Comment 4
2009-01-30 14:53:26 PST
I've attached our first stab. It's broken. We're now correctly returning a good VisualPosition right before the editable div, but canonicalPosition is turning our returned position into the next div. I'm not sure what changes we need to make to canonicalPosition to make this work. Should our placement be a "candidate position"? Should we special case to check if next's editibitility is not the same as position's? (That's probably the fix.) Here's the function for reference: In our case, we fall out the last return where it does return next; Position VisiblePosition::canonicalPosition(const Position& position) { // FIXME (9535): Canonicalizing to the leftmost candidate means that if we're at a line wrap, we will // ask renderers to paint downstream carets for other renderers. // To fix this, we need to either a) add code to all paintCarets to pass the responsibility off to // the appropriate renderer for VisiblePosition's like these, or b) canonicalize to the rightmost candidate // unless the affinity is upstream. Node* node = position.node(); if (!node) return Position(); node->document()->updateLayoutIgnorePendingStylesheets(); Position candidate = position.upstream(); if (candidate.isCandidate()) return candidate; candidate = position.downstream(); if (candidate.isCandidate()) return candidate; // When neither upstream or downstream gets us to a candidate (upstream/downstream won't leave // blocks or enter new ones), we search forward and backward until we find one. Position next = canonicalizeCandidate(nextCandidate(position)); Position prev = canonicalizeCandidate(previousCandidate(position)); Node* nextNode = next.node(); Node* prevNode = prev.node(); // The new position must be in the same editable element. Enforce that first. // Unless the descent is from a non-editable html element to an editable body. if (node->hasTagName(htmlTag) && !node->isContentEditable()) return next.isNotNull() ? next : prev; Node* editingRoot = editableRootForPosition(position); // If the html element is editable, descending into its body will look like a descent // from non-editable to editable content since rootEditableElement() always stops at the body. if (editingRoot && editingRoot->hasTagName(htmlTag) || position.node()->isDocumentNode()) return next.isNotNull() ? next : prev; bool prevIsInSameEditableElement = prevNode && editableRootForPosition(prev) == editingRoot; bool nextIsInSameEditableElement = nextNode && editableRootForPosition(next) == editingRoot; if (prevIsInSameEditableElement && !nextIsInSameEditableElement) return prev; if (nextIsInSameEditableElement && !prevIsInSameEditableElement) return next; if (!nextIsInSameEditableElement && !prevIsInSameEditableElement) return Position(); // The new position should be in the same block flow element. Favor that. Node *originalBlock = node->enclosingBlockFlowElement(); bool nextIsOutsideOriginalBlock = !nextNode->isDescendantOf(originalBlock) && nextNode != originalBlock; bool prevIsOutsideOriginalBlock = !prevNode->isDescendantOf(originalBlock) && prevNode != originalBlock; if (nextIsOutsideOriginalBlock && !prevIsOutsideOriginalBlock) return prev; return next; }
Eric Seidel (no email)
Comment 5
2009-01-30 14:53:55 PST
I'm very interested in Justin's comment here.
Eric Seidel (no email)
Comment 6
2009-01-30 15:01:45 PST
I was wrong! We hit this line: if (!nextIsInSameEditableElement && !prevIsInSameEditableElement) return Position(); We might just change that to return "position" instead.
Eric Seidel (no email)
Comment 7
2009-01-30 15:10:24 PST
Yay! Now we're crashing at: if (previous.isNull()) { ASSERT_NOT_REACHED(); in Selection::adjustForEditableContent.. sigh.
Eric Seidel (no email)
Comment 8
2009-01-30 16:33:07 PST
Ha! This is a regression:
http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/editing/selection/contenteditable-click-outside-expected.png
We already had a test which was failing!
Eric Seidel (no email)
Comment 9
2009-01-30 17:54:30 PST
Created
attachment 27204
[details]
Fix bugs 23605 and 23607 LayoutTests/ChangeLog | 33 +++++ ...ick-in-margins-inside-editable-div-expected.txt | 23 ++++ .../click-in-margins-inside-editable-div.html | 13 ++ .../click-outside-editable-div-expected.txt | 17 +++ .../selection/click-outside-editable-div.html | 13 ++ .../selection/contenteditable-click-outside.html | 13 -- .../editing/selection/resources/TEMPLATE.html | 13 ++ .../click-in-margins-inside-editable-div.js | 138 ++++++++++++++++++++ .../resources/click-outside-editable-div.js | 50 +++++++ ...contenteditable-click-outside-expected.checksum | 1 - .../contenteditable-click-outside-expected.png | Bin 21262 -> 0 bytes .../contenteditable-click-outside-expected.txt | 16 --- .../selection/select-all-iframe-expected.txt | 4 +- .../select-from-textfield-outwards-expected.txt | 2 +- .../contenteditable-click-outside-expected.txt | 12 -- .../selection/select-all-iframe-expected.txt | 56 -------- WebCore/ChangeLog | 28 ++++ WebCore/editing/VisiblePosition.cpp | 4 +- WebCore/rendering/RenderBlock.cpp | 115 +++++++--------- WebCore/rendering/RenderContainer.cpp | 3 +- WebCore/rendering/RenderInline.cpp | 1 + 21 files changed, 385 insertions(+), 170 deletions(-)
Eric Seidel (no email)
Comment 10
2009-01-30 17:56:27 PST
The editing delegate changes are not well understood, but I think they're progressions. Perhaps when Justing reviews this he can comment on if they look better to him too! :)
Eric Seidel (no email)
Comment 11
2009-01-30 18:26:21 PST
Created
attachment 27207
[details]
Fix bugs 23605 and 23607 LayoutTests/ChangeLog | 33 +++++ ...ick-in-margins-inside-editable-div-expected.txt | 23 ++++ .../click-in-margins-inside-editable-div.html | 13 ++ .../click-outside-editable-div-expected.txt | 17 +++ .../selection/click-outside-editable-div.html | 13 ++ .../selection/contenteditable-click-outside.html | 13 -- .../editing/selection/resources/TEMPLATE.html | 13 ++ .../click-in-margins-inside-editable-div.js | 138 ++++++++++++++++++++ .../resources/click-outside-editable-div.js | 50 +++++++ ...contenteditable-click-outside-expected.checksum | 1 - .../contenteditable-click-outside-expected.png | Bin 21262 -> 0 bytes .../contenteditable-click-outside-expected.txt | 16 --- .../selection/select-all-iframe-expected.txt | 4 +- .../select-from-textfield-outwards-expected.txt | 2 +- .../contenteditable-click-outside-expected.txt | 12 -- .../selection/select-all-iframe-expected.txt | 56 -------- WebCore/ChangeLog | 28 ++++ WebCore/editing/VisiblePosition.cpp | 4 +- WebCore/rendering/RenderBlock.cpp | 113 +++++++--------- WebCore/rendering/RenderContainer.cpp | 3 +- WebCore/rendering/RenderInline.cpp | 1 + 21 files changed, 383 insertions(+), 170 deletions(-)
Eric Seidel (no email)
Comment 12
2009-01-30 22:12:58 PST
Created
attachment 27210
[details]
Fix bugs 23605 and 23607 LayoutTests/ChangeLog | 33 +++++ ...ick-in-margins-inside-editable-div-expected.txt | 23 ++++ .../click-in-margins-inside-editable-div.html | 13 ++ .../click-outside-editable-div-expected.txt | 17 +++ .../selection/click-outside-editable-div.html | 13 ++ .../selection/contenteditable-click-outside.html | 13 -- .../editing/selection/resources/TEMPLATE.html | 13 ++ .../click-in-margins-inside-editable-div.js | 138 ++++++++++++++++++++ .../resources/click-outside-editable-div.js | 50 +++++++ ...contenteditable-click-outside-expected.checksum | 1 - .../contenteditable-click-outside-expected.png | Bin 21262 -> 0 bytes .../contenteditable-click-outside-expected.txt | 16 --- .../selection/select-all-iframe-expected.txt | 4 +- .../select-from-textfield-outwards-expected.txt | 2 +- .../contenteditable-click-outside-expected.txt | 12 -- .../selection/select-all-iframe-expected.txt | 56 -------- WebCore/ChangeLog | 28 ++++ WebCore/editing/VisiblePosition.cpp | 4 +- WebCore/rendering/RenderBlock.cpp | 113 +++++++--------- WebCore/rendering/RenderContainer.cpp | 3 +- WebCore/rendering/RenderInline.cpp | 1 + 21 files changed, 383 insertions(+), 170 deletions(-)
Eric Seidel (no email)
Comment 13
2009-01-30 22:14:59 PST
Created
attachment 27211
[details]
Fix bugs 23605 and 23607 LayoutTests/ChangeLog | 33 ++++ ...ick-in-margins-inside-editable-div-expected.txt | 23 +++ .../click-in-margins-inside-editable-div.html | 13 ++ .../click-outside-editable-div-expected.txt | 17 ++ .../selection/click-outside-editable-div.html | 13 ++ .../selection/contenteditable-click-outside.html | 13 -- .../editing/selection/resources/TEMPLATE.html | 13 ++ .../click-in-margins-inside-editable-div.js | 138 ++++++++++++++++ .../resources/click-outside-editable-div.js | 50 ++++++ ...contenteditable-click-outside-expected.checksum | 1 - .../contenteditable-click-outside-expected.png | Bin 21262 -> 0 bytes .../contenteditable-click-outside-expected.txt | 16 -- .../selection/select-all-iframe-expected.txt | 4 +- .../select-from-textfield-outwards-expected.txt | 2 +- .../contenteditable-click-outside-expected.txt | 12 -- .../selection/select-all-iframe-expected.txt | 56 ------- WebCore/ChangeLog | 31 ++++ WebCore/editing/VisiblePosition.cpp | 4 +- WebCore/rendering/RenderBlock.cpp | 174 +++++++++----------- WebCore/rendering/RenderContainer.cpp | 3 +- WebCore/rendering/RenderInline.cpp | 1 + 21 files changed, 419 insertions(+), 198 deletions(-)
Justin Garcia
Comment 14
2009-02-02 17:35:08 PST
> The editing delegate changes are not well understood
The change in: /LayoutTests/platform/mac/editing/selection/select-all-iframe-expected.txt comes from your fix for 23605 (the x position of the caret that's created is close to where the mouseDown occurred now instead of after the line of text). So that's fine. But, the change in: /LayoutTests/platform/mac/editing/selection/select-from-textfield-outwards-expected.txt leads me to believe that the patch may have caused a regression in behavior when you click inside a text field and drag-select up. It looks like for a particular position above the textfield, we create the wrong selection. Check out line 25 of select-from-textfield-outwards: eventSender.mouseMoveTo(middleX, middleY - field.offsetHeight); It looks like that used to correspond to this: 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 which is being replaced by: EDITING 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 which doesn't seem right. Also I think: +// FIXME: This function should could go on RenderObject an instance method then +// all cases in which positionForCoordinates recurses could call this instead to should perhaps read "...should go in an RenderObject instance method, then...".
Eric Seidel (no email)
Comment 15
2009-02-02 17:58:26 PST
Created
attachment 27270
[details]
A full git patch for easier applying by ojan (with git am)
Ojan Vafai
Comment 16
2009-02-05 17:42:18 PST
Created
attachment 27371
[details]
Fix positionForCoordinates now with an added fix for clicking in the margins of the body. LayoutTests/ChangeLog | 31 ++++ ...ick-in-margins-inside-editable-div-expected.txt | 23 +++ .../click-in-margins-inside-editable-div.html | 13 ++ .../click-outside-editable-div-expected.txt | 20 +++ .../selection/click-outside-editable-div.html | 13 ++ .../selection/contenteditable-click-outside.html | 13 -- .../editing/selection/resources/TEMPLATE.html | 13 ++ .../click-in-margins-inside-editable-div.js | 138 ++++++++++++++++ .../resources/click-outside-editable-div.js | 55 ++++++ .../editing/selection/select-missing-image.html | 2 +- ...contenteditable-click-outside-expected.checksum | 1 - .../contenteditable-click-outside-expected.txt | 16 -- .../selection/select-all-iframe-expected.txt | 4 +- .../select-from-textfield-outwards-expected.txt | 2 +- .../selection/select-missing-image-expected.txt | 3 +- .../contenteditable-click-outside-expected.txt | 12 -- .../selection/select-all-iframe-expected.txt | 56 ------- WebCore/ChangeLog | 29 ++++ WebCore/editing/VisiblePosition.cpp | 6 +- WebCore/rendering/RenderBlock.cpp | 174 +++++++++----------- WebCore/rendering/RenderInline.cpp | 1 + 21 files changed, 426 insertions(+), 199 deletions(-)
Ojan Vafai
Comment 17
2009-02-05 17:50:43 PST
WRT LayoutTests/platform/mac/editing/selection/select-from-textfield-outwards-expected.txt, I think the new behavior is very similar to the old behavior. Specifically, they both have a bug where if you click from the middle of a textfield and drag upwards, at first it selects to the end of the textfield, then at some point selects to the beginning (from the middle). The only difference is where the switch happens. In the old code, it's somewhere in the middle of the margin. In the new code, it's at the top of the margin. This is a separate bug that should be fixed so that we always select from the middle to the beginning of the textfield, or that we do the IE behavior and ignore that you're above the field and continue selecting text as you drag in the x-axis. WRT LayoutTests/editing/selection/select-missing-image.html. It relied on the old buggy behavior. A execCommand(SelectAll) on a selection that *contains* a contentEditable div should not select the contents of the div. So the new behavior is more correct. My change to the test is to give it an ID that matches the rest of the selection tests, which correctly puts focus inside the div before calling the execCommand. It still tests the original goal that a missing image properly shows it's selected.
Dave Hyatt
Comment 18
2009-02-10 17:48:10 PST
Comment on
attachment 27371
[details]
Fix positionForCoordinates now with an added fix for clicking in the margins of the body.
> --- a/WebCore/editing/VisiblePosition.cpp > +++ b/WebCore/editing/VisiblePosition.cpp > @@ -478,7 +478,7 @@ Position VisiblePosition::canonicalPosition(const Position& position) > > // The new position must be in the same editable element. Enforce that first. > // Unless the descent is from a non-editable html element to an editable body. > - if (node->hasTagName(htmlTag) && !node->isContentEditable()) > + if (node->hasTagName(htmlTag) && !node->isContentEditable() && node->ownerDocument()->body()->isContentEditable()) > return next.isNotNull() ? next : prev; >
I think body() can be null in certain circumstances but am not positive.
> +// FIXME: This function should could go on RenderObject an instance method then > +// all cases in which positionForCoordinates recurses could call this instead to > +// prevent crossing editable boundaries. This would require many tests!
Can you clean up the comment? "should could" and did you mean "as an instance method"? I think "recurs" is easier to read than "recurses". Same for other places where you say "recurse" (I'd just say "recur.")
> +static VisiblePosition positionForPointRespectingEditingBoundaries(RenderBox* parent, RenderBox* child, const IntPoint& parentCoords) > +{ > + int xInChildCoords = parentCoords.x() - child->x(); > + int yInChildCoords = parentCoords.y() - child->y(); > + > + // If this is an anonymous renderer, we just recurse normally > + Node* childNode = child->element(); > + if (!childNode) > + return child->positionForCoordinates(xInChildCoords, yInChildCoords); > + > + // Otherwise, first make sure that the editability of the parent and child agree!
I don't think the ! is necessary here. I mean, it's just editing code. It's not *that* exciting.
> + // If they don't agree, then we return a visible position just before or after the child > + RenderObject* ancestor = parent; > + while (ancestor && !ancestor->element()) > + ancestor = ancestor->parent(); > + > + // If we can't find an ancestor to check editability on, or editibility is unchanged, we recurse like normal
Typo. "editibility" should be "editability"
> +VisiblePosition positionForPointWithInlineChildren(RenderBlock* block, const IntPoint& pointInContents, int xInParentCoords)
Should be static.
> +{ > + ASSERT(block->childrenInline()); > + > + if (!block->firstRootBox()) > + return VisiblePosition(block->element(), 0, DOWNSTREAM); > + > + // look for the closest line box in the root box which is at the passed-in y coordinate > + for (RootInlineBox* root = block->firstRootBox(); root; root = root->nextRootBox()) { > + int bottom; > + // set the bottom based on whether there is a next root box > + if (root->nextRootBox()) > + // FIXME: make the break point halfway between the bottom of the previous root box and the top of the next root box > + bottom = root->nextRootBox()->topOverflow(); > + else > + bottom = root->bottomOverflow() + verticalLineClickFudgeFactor; > + // check if this root line box is located at this y coordinate > + if (pointInContents.y() < bottom && root->firstChild()) { > + // FIXME: It seems like a bug that this function needs xInParentCoords > + InlineBox* closestBox = root->closestLeafChildForXPos(xInParentCoords);
It does indeed seem like a bug at first glance. Please make sure to split out a followup bug on this issue.
> diff --git a/WebCore/rendering/RenderInline.cpp b/WebCore/rendering/RenderInline.cpp > index 8f09fc7..e8ee55d 100644 > --- a/WebCore/rendering/RenderInline.cpp > +++ b/WebCore/rendering/RenderInline.cpp > @@ -485,6 +485,7 @@ VisiblePosition RenderInline::positionForCoordinates(int x, int y) > while (c) { > RenderBox* contBlock = c; > if (c->isInline() || c->firstChild()) > + // The continuations (anonymous blocks) will never be editable, so we don't need to worry about crossing editable boundaries here. > return c->positionForCoordinates(parentBlockX - contBlock->x(), parentBlockY - contBlock->y()); > c = toRenderBlock(c)->inlineContinuation(); > }
Just revert this change please. I think the comment here is more confusing than helpful.
Ojan Vafai
Comment 19
2009-02-12 16:02:03 PST
Created
attachment 27626
[details]
Fix for clicking below the last root line box with multiple line boxes. LayoutTests/ChangeLog | 35 ++++ ...ick-in-margins-inside-editable-div-expected.txt | 23 +++ .../click-in-margins-inside-editable-div.html | 14 ++ ...n-padding-with-multiple-line-boxes-expected.txt | 11 ++ .../click-in-padding-with-multiple-line-boxes.html | 14 ++ .../click-outside-editable-div-expected.txt | 20 ++ .../selection/click-outside-editable-div.html | 14 ++ .../selection/contenteditable-click-outside.html | 13 -- .../editing/selection/resources/TEMPLATE.html | 14 ++ .../click-in-margins-inside-editable-div.js | 93 ++++++++++ .../click-in-padding-with-multiple-line-boxes.js | 62 +++++++ .../resources/click-outside-editable-div.js | 49 +++++ .../resources/js-test-selection-shared.js | 44 +++++ .../editing/selection/select-missing-image.html | 2 +- ...contenteditable-click-outside-expected.checksum | 1 - .../contenteditable-click-outside-expected.txt | 16 -- .../selection/select-all-iframe-expected.txt | 4 +- .../select-from-textfield-outwards-expected.txt | 2 +- .../selection/select-missing-image-expected.txt | 3 +- .../contenteditable-click-outside-expected.txt | 12 -- .../selection/select-all-iframe-expected.txt | 56 ------ WebCore/ChangeLog | 29 +++ WebCore/editing/VisiblePosition.cpp | 10 +- WebCore/rendering/RenderBlock.cpp | 184 ++++++++++---------- 24 files changed, 525 insertions(+), 200 deletions(-)
Ojan Vafai
Comment 20
2009-02-12 16:06:37 PST
All addressed. Two issues: 1. If the body was null, I wasn't really sure what to do. returning Position() seems like the only option though. All the layout tests still pass, but I doubt we have any that test a null body. 2. I just went ahead and fixed the FIXME you wanted me to file a bug for. Added a layout test for that case as well. In the process, found a bug this code was about to introduce. :)
Eric Seidel (no email)
Comment 21
2009-02-13 11:27:02 PST
***
Bug 23605
has been marked as a duplicate of this bug. ***
Dave Hyatt
Comment 22
2009-02-17 14:48:22 PST
Comment on
attachment 27626
[details]
Fix for clicking below the last root line box with multiple line boxes. r=me
Ojan Vafai
Comment 23
2009-02-24 13:25:45 PST
Created
attachment 27927
[details]
Same patch, but synced to trunk for easier landing by eseidel. LayoutTests/ChangeLog | 35 ++++ ...ick-in-margins-inside-editable-div-expected.txt | 23 +++ .../click-in-margins-inside-editable-div.html | 14 ++ ...n-padding-with-multiple-line-boxes-expected.txt | 11 ++ .../click-in-padding-with-multiple-line-boxes.html | 14 ++ .../click-outside-editable-div-expected.txt | 20 ++ .../selection/click-outside-editable-div.html | 14 ++ .../selection/contenteditable-click-outside.html | 13 -- .../editing/selection/resources/TEMPLATE.html | 14 ++ .../click-in-margins-inside-editable-div.js | 93 ++++++++++ .../click-in-padding-with-multiple-line-boxes.js | 62 +++++++ .../resources/click-outside-editable-div.js | 49 +++++ .../resources/js-test-selection-shared.js | 44 +++++ .../editing/selection/select-missing-image.html | 2 +- ...contenteditable-click-outside-expected.checksum | 1 - .../contenteditable-click-outside-expected.txt | 16 -- .../selection/select-all-iframe-expected.txt | 4 +- .../select-from-textfield-outwards-expected.txt | 2 +- .../selection/select-missing-image-expected.txt | 3 +- .../contenteditable-click-outside-expected.txt | 12 -- .../selection/select-all-iframe-expected.txt | 56 ------ WebCore/ChangeLog | 29 +++ WebCore/editing/VisiblePosition.cpp | 10 +- WebCore/rendering/RenderBlock.cpp | 184 ++++++++++---------- 24 files changed, 525 insertions(+), 200 deletions(-)
Ojan Vafai
Comment 24
2009-02-24 13:32:51 PST
Created
attachment 27929
[details]
patch made with git format-patch for even easier patching by eseidel. :)
Eric Seidel (no email)
Comment 25
2009-02-24 14:28:44 PST
Committing to
http://svn.webkit.org/repository/webkit/trunk
... D LayoutTests/editing/selection/contenteditable-click-outside.html D LayoutTests/platform/mac/editing/selection/contenteditable-click-outside-expected.checksum D LayoutTests/platform/mac/editing/selection/contenteditable-click-outside-expected.txt D LayoutTests/platform/qt/editing/selection/contenteditable-click-outside-expected.txt D LayoutTests/platform/qt/editing/selection/select-all-iframe-expected.txt M LayoutTests/ChangeLog A LayoutTests/editing/selection/click-in-margins-inside-editable-div-expected.txt A LayoutTests/editing/selection/click-in-margins-inside-editable-div.html A LayoutTests/editing/selection/click-in-padding-with-multiple-line-boxes-expected.txt A LayoutTests/editing/selection/click-in-padding-with-multiple-line-boxes.html A LayoutTests/editing/selection/click-outside-editable-div-expected.txt A LayoutTests/editing/selection/click-outside-editable-div.html A LayoutTests/editing/selection/resources/TEMPLATE.html A LayoutTests/editing/selection/resources/click-in-margins-inside-editable-div.js A LayoutTests/editing/selection/resources/click-in-padding-with-multiple-line-boxes.js A LayoutTests/editing/selection/resources/click-outside-editable-div.js A LayoutTests/editing/selection/resources/js-test-selection-shared.js M LayoutTests/editing/selection/select-missing-image.html M LayoutTests/platform/mac/editing/selection/select-all-iframe-expected.txt M LayoutTests/platform/mac/editing/selection/select-from-textfield-outwards-expected.txt M LayoutTests/platform/mac/editing/selection/select-missing-image-expected.txt M WebCore/ChangeLog M WebCore/editing/VisiblePosition.cpp M WebCore/rendering/RenderBlock.cpp Committed
r41191
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug