[Webkit-unassigned] [Bug 25002] Repeated copy/paste can lead to deeply nested divs
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 1 10:35:07 PST 2010
https://bugs.webkit.org/show_bug.cgi?id=25002
--- Comment #3 from Enrica Casucci <enrica at apple.com> 2010-02-01 10:35:06 PST ---
(From update of attachment 47813)
> diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
> index b3b56b3..7a80056 100644
> --- a/LayoutTests/ChangeLog
> +++ b/LayoutTests/ChangeLog
> @@ -1,3 +1,25 @@
> +2010-01-31 Tony Chang <tony at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + https://bugs.webkit.org/show_bug.cgi?id=25002
> + When inserting a new paragraph, avoid nesting empty divs. When
> + pasting near the end of a paragraph, this prevents each paste
> + command for getting nested one level deeper.
> +
> + Three paste tests have been rebaselined since this causes the pasted
> + content to be outside the last div instead of inside. E.g.,
> + <div>foo<div>bar</div>[pasted content]</div> is now
> + <div>foo<div>bar</div></div><div>[pasted content]</div>
> +
> + The new test verifies this behavior.
> +
> + * editing/inserting/paragraph-outside-nested-divs-expected.txt: Added.
> + * editing/inserting/paragraph-outside-nested-divs.html: Added.
> + * platform/mac/editing/pasteboard/paste-text-012-expected.txt:
> + * platform/mac/editing/pasteboard/paste-text-013-expected.txt:
> + * platform/mac/editing/pasteboard/paste-text-017-expected.txt:
> +
> 2010-01-31 Kent Tamura <tkent at chromium.org>
>
> Reviewed by Darin Adler.
> diff --git a/LayoutTests/editing/inserting/paragraph-outside-nested-divs-expected.txt b/LayoutTests/editing/inserting/paragraph-outside-nested-divs-expected.txt
> new file mode 100644
> index 0000000..7f1d2d5
> --- /dev/null
> +++ b/LayoutTests/editing/inserting/paragraph-outside-nested-divs-expected.txt
> @@ -0,0 +1,9 @@
> +When inserting a new line, we should break out of nested divs.
> +
> +SUCCESS
> +
> +first
> +second
> +This should be in nested divs.
> +third
> +This should be in a single div.
> diff --git a/LayoutTests/editing/inserting/paragraph-outside-nested-divs.html b/LayoutTests/editing/inserting/paragraph-outside-nested-divs.html
> new file mode 100644
> index 0000000..c366b6d
> --- /dev/null
> +++ b/LayoutTests/editing/inserting/paragraph-outside-nested-divs.html
> @@ -0,0 +1,45 @@
> +<body contentEditable="true">
> +<p>When inserting a new line, we should break out of nested divs.</p>
> +<p id="results"><p>
> +<div>first<div>second</div><div>third</div></div>
> +</body>
> +<script src="../editing.js"></script>
> +<script>
> + if (window.layoutTestController)
> + window.layoutTestController.dumpAsText();
> +
> + function fail(msg) {
> + document.getElementById("results").innerText = "FAIL";
> + throw msg;
> + }
> +
> + // Try inserting a new line after the last div.
> + var div2 = document.getElementsByTagName("div")[1];
> + var div3 = document.getElementsByTagName("div")[2];
> + execSetSelectionCommand(div3, 0, div3, 0);
> + execMoveSelectionForwardByWordCommand();
> +
> + execTypeCharacterCommand("\n");
> + execInsertHTMLCommand("This should be in a single div.");
> +
> + var div4 = document.getElementsByTagName("div")[3];
> + if (div4.innerHTML != "This should be in a single div.")
> + fail("wrong div4? " + div4.innerHTML);
> + if (div4.parentNode.tagName != "BODY")
> + fail("div should not be nested: " + div4.parentNode.tagName);
> +
> + // Try inserting a new line after the second div. This should be nested.
> + execSetSelectionCommand(div2, 0, div2, 0);
> + execMoveSelectionForwardByWordCommand();
> +
> + execTypeCharacterCommand("\n");
> + execInsertHTMLCommand("This should be in nested divs.");
> +
> + var nestedDiv = document.getElementsByTagName("div")[2];
> + if (nestedDiv.innerHTML != "This should be in nested divs.")
> + fail("wrong nestedDiv? " + nestedDiv.innerHTML);
> + if (nestedDiv.parentNode.tagName != "DIV")
> + fail("div should be nested: " + nestedDiv.parentNode.tagName);
> +
> + document.getElementById("results").innerText = "SUCCESS";
> +</script>
> diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-text-012-expected.txt b/LayoutTests/platform/mac/editing/pasteboard/paste-text-012-expected.txt
> index 429e300..7916c24 100644
> --- a/LayoutTests/platform/mac/editing/pasteboard/paste-text-012-expected.txt
> +++ b/LayoutTests/platform/mac/editing/pasteboard/paste-text-012-expected.txt
> @@ -7,7 +7,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
> -EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > DIV > SPAN > DIV > BODY > HTML > #document to 0 of DIV > DIV > DIV > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> +EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > SPAN > DIV > BODY > HTML > #document to 0 of DIV > DIV > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
> layer at (0,0) size 800x600
> @@ -36,12 +36,12 @@ layer at (0,0) size 800x600
> RenderInline {SPAN} at (0,0) size 0x0
> RenderBlock (anonymous) at (14,14) size 756x132
> RenderBlock {DIV} at (0,0) size 756x132 [border: (2px solid #FF0000)]
> - RenderBlock {DIV} at (14,38) size 728x80
> + RenderBlock {DIV} at (14,38) size 728x28
> RenderBlock {BLOCKQUOTE} at (40,0) size 648x28
> RenderText {#text} at (0,0) size 32x28
> text run at (0,0) width 32: "foo"
> - RenderBlock {DIV} at (0,52) size 728x28
> - RenderBR {BR} at (0,0) size 0x28
> + RenderBlock {DIV} at (14,90) size 728x28
> + RenderBR {BR} at (0,0) size 0x28
> RenderBlock (anonymous) at (14,146) size 756x0
> RenderInline {SPAN} at (0,0) size 0x0
> -caret: position 0 of child 0 {BR} of child 1 {DIV} of child 0 {DIV} of child 0 {DIV} of child 0 {SPAN} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
> +caret: position 0 of child 0 {BR} of child 1 {DIV} of child 0 {DIV} of child 0 {SPAN} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
> diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-text-013-expected.txt b/LayoutTests/platform/mac/editing/pasteboard/paste-text-013-expected.txt
> index 3c1bbda..b0bd9e1 100644
> --- a/LayoutTests/platform/mac/editing/pasteboard/paste-text-013-expected.txt
> +++ b/LayoutTests/platform/mac/editing/pasteboard/paste-text-013-expected.txt
> @@ -10,7 +10,7 @@ EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
> EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 1 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
> -EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> +EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
> layer at (0,0) size 800x600
> @@ -40,10 +40,10 @@ layer at (0,0) size 800x600
> RenderText {#text} at (14,14) size 12x28
> text run at (14,14) width 12: "x"
> RenderBlock {DIV} at (0,238) size 784x132 [border: (2px solid #FF0000)]
> - RenderBlock {DIV} at (14,38) size 756x80
> + RenderBlock {DIV} at (14,38) size 756x28
> RenderBlock {BLOCKQUOTE} at (40,0) size 676x28
> RenderText {#text} at (0,0) size 32x28
> text run at (0,0) width 32: "foo"
> - RenderBlock {DIV} at (0,52) size 756x28
> - RenderBR {BR} at (0,0) size 0x28
> -caret: position 0 of child 0 {BR} of child 1 {DIV} of child 0 {DIV} of child 8 {DIV} of child 1 {BODY} of child 0 {HTML} of document
> + RenderBlock {DIV} at (14,90) size 756x28
> + RenderBR {BR} at (0,0) size 0x28
> +caret: position 0 of child 0 {BR} of child 1 {DIV} of child 8 {DIV} of child 1 {BODY} of child 0 {HTML} of document
> diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-text-017-expected.txt b/LayoutTests/platform/mac/editing/pasteboard/paste-text-017-expected.txt
> index b1fff1c..af6d2a0 100644
> --- a/LayoutTests/platform/mac/editing/pasteboard/paste-text-017-expected.txt
> +++ b/LayoutTests/platform/mac/editing/pasteboard/paste-text-017-expected.txt
> @@ -6,7 +6,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 5 of #text > DIV > DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> -EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > DIV > DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> +EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
> EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
> EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
> layer at (0,0) size 800x600
> @@ -40,14 +40,14 @@ layer at (0,0) size 800x600
> text run at (0,0) width 35: "one"
> RenderBlock {DIV} at (2,30) size 780x28
> RenderBR {BR} at (0,0) size 0x28
> - RenderBlock {DIV} at (2,58) size 780x56
> + RenderBlock {DIV} at (2,58) size 780x28
> RenderBlock {DIV} at (0,0) size 780x28
> RenderText {#text} at (0,0) size 36x28
> text run at (0,0) width 36: "two"
> RenderBlock (anonymous) at (0,28) size 780x0
> - RenderBlock {DIV} at (0,28) size 780x28
> - RenderBR {BR} at (0,0) size 0x28
> + RenderBlock {DIV} at (2,86) size 780x28
> + RenderBR {BR} at (0,0) size 0x28
> RenderBlock {DIV} at (2,114) size 780x28
> RenderText {#text} at (0,0) size 49x28
> text run at (0,0) width 49: "three"
> -caret: position 0 of child 0 {BR} of child 1 {DIV} of child 5 {DIV} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
> +caret: position 0 of child 0 {BR} of child 6 {DIV} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 240f69c..d7d1e72 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,18 @@
> +2010-01-31 Tony Chang <tony at chromium.org>
> +
> + Reviewed by NOBODY (OOPS!).
> +
> + https://bugs.webkit.org/show_bug.cgi?id=25002
> + When inserting a new paragraph, avoid nesting empty divs. When
> + pasting near the end of a paragraph, this prevents each paste
> + command for getting nested one level deeper.
> +
> + Test: editing/inserting/paragraph-outside-nested-divs.html
> +
> + * editing/InsertParagraphSeparatorCommand.cpp:
> + (WebCore::findInsertionSibling):
> + (WebCore::InsertParagraphSeparatorCommand::doApply):
> +
> 2010-01-31 Kent Tamura <tkent at chromium.org>
>
> Reviewed by Darin Adler.
> diff --git a/WebCore/editing/InsertParagraphSeparatorCommand.cpp b/WebCore/editing/InsertParagraphSeparatorCommand.cpp
> index 695f46a..75b85a3 100644
> --- a/WebCore/editing/InsertParagraphSeparatorCommand.cpp
> +++ b/WebCore/editing/InsertParagraphSeparatorCommand.cpp
> @@ -44,6 +44,25 @@ namespace WebCore {
>
> using namespace HTMLNames;
>
> +// When inserting a new line, we want to avoid nesting empty divs if we can. Otherwise, when
> +// pasting, it's easy to have each new line be a div deeper than the previous. E.g., in the case
> +// below, we want to insert at ^ instead of |.
> +// <div>foo<div>bar</div>|</div>^
> +static Element* findInsertionSibling(Element* startBlock, const Element* blockToInsert)
> +{
> + if (!blockToInsert->hasTagName(divTag))
> + return startBlock;
> +
> + Element* curBlock = startBlock;
> + while (!curBlock->nextSibling() && curBlock->parentElement()->hasTagName(divTag)) {
> + NamedNodeMap* attributes = curBlock->parentElement()->attributes(true);
> + if (attributes && !attributes->isEmpty())
> + break;
> + curBlock = curBlock->parentElement();
> + }
> + return curBlock;
> +}
> +
> InsertParagraphSeparatorCommand::InsertParagraphSeparatorCommand(Document *document, bool mustUseDefaultParagraphElement)
> : CompositeEditCommand(document)
> , m_mustUseDefaultParagraphElement(mustUseDefaultParagraphElement)
> @@ -214,7 +233,11 @@ void InsertParagraphSeparatorCommand::doApply()
> // When inserting the newline after the blockquote, we don't want to apply the original style after the insertion
> shouldApplyStyleAfterInsertion = false;
> }
> - insertNodeAfter(blockToInsert, startBlock);
> +
> + // Most of the time we want to stay at the nesting level of the startBlock (e.g., when nesting within lists). However,
> + // for div nodes, this can result in nested div tags that are hard to break out of.
> + Element* siblingNode = findInsertionSibling(startBlock, blockToInsert.get());
> + insertNodeAfter(blockToInsert, siblingNode);
> }
>
> // Recreate the same structure in the new paragraph.
The change looks ok to me. I don't really like the name of the new method. You
should come up with something more descriptive.
What you are looking for is a visually equivalent position that avoids the
nesting.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list