Full Text Bug Listing

Annie Sullivan

Reported 2009-03-26 19:46:19 PDT

Steps to reproduce: Create a bulleted list: * a * b * c * Select "b" and "c" and copy. Place cursor at start of empty list item and paste. Actual result: New indented list is created under blank list item: * a * b * c * o b o c Expected result: List pastes at same indentation level: * a * b * c * b * c

Tony Chang

Comment 1 2010-02-02 21:15:02 PST

Created attachment 47992 [details] Patch

Tony Chang

Comment 2 2010-02-02 21:17:04 PST

(In reply to comment #1) > Created an attachment (id=47992) [details] > Patch This handles copy/paste of list items at any of indent level and seems to match Firefox and IE's behavior in this regard. There's still some work to be done to handle the split list item case (if you paste a list into the middle of text on a list item), but I think it's better to do one thing at a time.

Enrica Casucci

Comment 3 2010-02-03 10:38:24 PST

Comment on attachment 47992 [details] Patch > diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog > index 6b5ab6f..d54a47e 100644 > --- a/LayoutTests/ChangeLog > +++ b/LayoutTests/ChangeLog > @@ -1,3 +1,14 @@ > +2010-02-02 Tony Chang <tony@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + https://bugs.webkit.org/show_bug.cgi?id=24872 > + Add a test to make sure copying from a list and pasting into a list > + keeps the list at the same indention level rather than nesting. > + > + * editing/pasteboard/paste-list-002-expected.txt: Added. > + * editing/pasteboard/paste-list-002.html: Added. > + > 2010-02-02 Eric Seidel <eric@webkit.org> > > Reviewed by Gustavo Noronha Silva. > diff --git a/LayoutTests/editing/pasteboard/paste-list-002-expected.txt b/LayoutTests/editing/pasteboard/paste-list-002-expected.txt > new file mode 100644 > index 0000000..5ff1cb6 > --- /dev/null > +++ b/LayoutTests/editing/pasteboard/paste-list-002-expected.txt > @@ -0,0 +1,13 @@ > +Copy/pasting list items in a list. This test should be run with DRT for pasteboard access. > + > +PASS: <li>alpha</li><ul><li>beta</li><li>gamma</li></ul><li>beta</li><li>gamma</li><li id="delta">delta</li><li>beta</li><li>gamma</li><li></li> > + > +alpha > +beta > +gamma > +beta > +gamma > +delta > +beta > +gamma > + > diff --git a/LayoutTests/editing/pasteboard/paste-list-002.html b/LayoutTests/editing/pasteboard/paste-list-002.html > new file mode 100644 > index 0000000..2fae410 > --- /dev/null > +++ b/LayoutTests/editing/pasteboard/paste-list-002.html > @@ -0,0 +1,63 @@ > +<html> > +<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> > +<script> > + > +function editingTest() { > + // Select "beta" and "gamma". > + moveSelectionForwardByLineCommand(); > + for (i = 0; i < 2; i++) > + extendSelectionForwardByLineCommand(); > + copyCommand(); > + > + // Paste with the cursor right before "delta". > + moveSelectionForwardByLineCommand(); > + pasteCommand(); > + > + // Verify that the cursor is in the right place (still before delta). > + var selection = window.getSelection(); > + if (selection.baseNode.parentNode != document.getElementById("delta") || > + selection.baseOffset != 0 || !selection.isCollapsed) > + throw "Wrong selection position on before paste."; > + > + // Paste with the cursor at the end of "delta". > + moveSelectionForwardByWordCommand(); > + pasteCommand(); > + > + // Verify that the cursor is in the right place (new list item after delta). > + var selection = window.getSelection(); > + if (!selection.isCollapsed || selection.baseNode.value != "") > + throw "Wrong selection position on after paste."; > +} > + > +</script> > +<body> > +<div contentEditable="true"> > +<p>Copy/pasting list items in a list. This test should be run with DRT for pasteboard access.</p> > +<p id="results">FAIL</p> > +<ul id="test"> > + <li>alpha</li> > + <li>beta</li> > + <li>gamma</li> > + <li id="delta">delta</li> > +</ul> > +</div> > + > +<script> > +if (window.layoutTestController) > + layoutTestController.dumpAsText(); > + > +var elem = document.getElementById("test"); > +var selection = window.getSelection(); > +selection.setPosition(elem, 0); > +editingTest(); > + > +// Rerun the test but have the source list be indented. > +document.getElementById("test").innerHTML = "<li>alpha</li><ul><li>beta</li><li>gamma</li></ul><li id='delta'>delta</li>"; > +selection.setPosition(elem, 0); > +editingTest(); > + > +document.getElementById("results").innerText = "PASS: " + document.getElementById("test").innerHTML; > +</script> > + > +</body> > +</html> > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > index 67f5b56..6f365d0 100644 > --- a/WebCore/ChangeLog > +++ b/WebCore/ChangeLog > @@ -1,3 +1,30 @@ > +2010-02-02 Tony Chang <tony@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + https://bugs.webkit.org/show_bug.cgi?id=24872 > + When pasting a list into another list should not indent another level. > + If the cursor is at the beginning of the line, it should insert the > + list items before the current list item. If the cursor is at the end > + of the line, it should insert the list items after the current list item. > + > + This matches Firefox and IE and makes the common activity of reordering a list > + work as expected. > + > + This also adds a small helper method (isListItem) to htmlediting.h. > + > + Test: editing/pasteboard/paste-list-002.html > + > + * editing/ReplaceSelectionCommand.cpp: > + (WebCore::ReplaceSelectionCommand::doApply): > + (WebCore::ReplaceSelectionCommand::insertNodeAtAndUpdateNodesInserted): > + (WebCore::ReplaceSelectionCommand::insertAsListItems): > + * editing/ReplaceSelectionCommand.h: > + * editing/htmlediting.cpp: > + (WebCore::isListItem): > + (WebCore::appendedSublist): > + * editing/htmlediting.h: > + > 2010-02-02 Steve Falkenburg <sfalken@apple.com> > > Reviewed by Darin Adler. > diff --git a/WebCore/editing/ReplaceSelectionCommand.cpp b/WebCore/editing/ReplaceSelectionCommand.cpp > index 85a4471..77d69d1 100644 > --- a/WebCore/editing/ReplaceSelectionCommand.cpp > +++ b/WebCore/editing/ReplaceSelectionCommand.cpp > @@ -752,9 +752,7 @@ void ReplaceSelectionCommand::doApply() > bool startIsInsideMailBlockquote = nearestMailBlockquote(insertionPos.node()); > > if ((selectionStartWasStartOfParagraph && selectionEndWasEndOfParagraph && !startIsInsideMailBlockquote) || > - startBlock == currentRoot || > - (startBlock && startBlock->renderer() && startBlock->renderer()->isListItem()) || > - selectionIsPlainText) > + startBlock == currentRoot || isListItem(startBlock) || selectionIsPlainText) > m_preventNesting = false; > > if (selection.isRange()) { > @@ -871,7 +869,7 @@ void ReplaceSelectionCommand::doApply() > RefPtr<Node> node = refNode->nextSibling(); > > fragment.removeNode(refNode); > - insertNodeAtAndUpdateNodesInserted(refNode, insertionPos); > + refNode = insertNodeAtAndUpdateNodesInserted(refNode, insertionPos); > > // Mutation events (bug 22634) may have already removed the inserted content > if (!refNode->inDocument()) > @@ -960,9 +958,15 @@ void ReplaceSelectionCommand::doApply() > if (selectionEndWasEndOfParagraph || !isEndOfParagraph(endOfInsertedContent) || next.isNull()) { > if (!isStartOfParagraph(endOfInsertedContent)) { > setEndingSelection(endOfInsertedContent); > - // Use a default paragraph element (a plain div) for the empty paragraph, using the last paragraph > - // block's style seems to annoy users. > - insertParagraphSeparator(true); > + Node* enclosingNode = enclosingBlock(endOfInsertedContent.deepEquivalent().node()); > + if (isListItem(enclosingNode)) { > + RefPtr<Node> newListItem = createListItemElement(document()); > + insertNodeAfter(newListItem, enclosingNode); > + setEndingSelection(VisiblePosition(Position(newListItem, 0))); > + } else > + // Use a default paragraph element (a plain div) for the empty paragraph, using the last paragraph > + // block's style seems to annoy users. > + insertParagraphSeparator(true); > > // Select up to the paragraph separator that was added. > lastPositionToSelect = endingSelection().visibleStart().deepEquivalent(); > @@ -1097,11 +1101,16 @@ void ReplaceSelectionCommand::insertNodeAfterAndUpdateNodesInserted(PassRefPtr<N > updateNodesInserted(nodeToUpdate); > } > > -void ReplaceSelectionCommand::insertNodeAtAndUpdateNodesInserted(PassRefPtr<Node> insertChild, const Position& p) > +Node* ReplaceSelectionCommand::insertNodeAtAndUpdateNodesInserted(PassRefPtr<Node> insertChild, const Position& p) > { > Node* nodeToUpdate = insertChild.get(); // insertChild will be cleared when passed > + Node* blockStart = enclosingBlock(p.node()); > + if (isListElement(insertChild.get()) && blockStart->renderer()->isListItem()) > + return insertAsListItems(insertChild, blockStart, p); > + > insertNodeAt(insertChild, p); > updateNodesInserted(nodeToUpdate); > + return nodeToUpdate; > } > > void ReplaceSelectionCommand::insertNodeBeforeAndUpdateNodesInserted(PassRefPtr<Node> insertChild, Node* refChild) > @@ -1111,6 +1120,39 @@ void ReplaceSelectionCommand::insertNodeBeforeAndUpdateNodesInserted(PassRefPtr< > updateNodesInserted(nodeToUpdate); > } > > +// If the user is inserting a list into an existing list, instead of nesting the list, > +// we put the list items into the existing list. > +Node* ReplaceSelectionCommand::insertAsListItems(PassRefPtr<Node> listElement, Node* insertionNode, const Position& p) > +{ > + while (listElement->hasChildNodes() && isListElement(listElement->firstChild()) && listElement->childNodeCount() == 1) > + listElement = listElement->firstChild(); > + > + bool isStart = isStartOfParagraph(p); > + bool isEnd = isEndOfParagraph(p); > + > + Node* lastNode = insertionNode; > + while (RefPtr<Node> listItem = listElement->firstChild()) { > + ExceptionCode ec = 0; > + listElement->removeChild(listItem.get(), ec); > + ASSERT(!ec); > + if (isStart) > + insertNodeBefore(listItem, lastNode); > + else if (isEnd) { > + insertNodeAfter(listItem, lastNode); > + lastNode = listItem.get(); > + } else { > + // FIXME: If we're in the middle of a list item, we should split it into two separate > + // list items and insert these nodes between them. For now, just append the nodes. > + insertNodeAfter(listItem, lastNode); > + lastNode = listItem.get(); > + } > + } > + if (isStart) > + lastNode = lastNode->previousSibling(); > + updateNodesInserted(lastNode); > + return lastNode; > +} > + > void ReplaceSelectionCommand::updateNodesInserted(Node *node) > { > if (!node) > diff --git a/WebCore/editing/ReplaceSelectionCommand.h b/WebCore/editing/ReplaceSelectionCommand.h > index 1cb93c3..b7ff857 100644 > --- a/WebCore/editing/ReplaceSelectionCommand.h > +++ b/WebCore/editing/ReplaceSelectionCommand.h > @@ -52,8 +52,9 @@ private: > void completeHTMLReplacement(const Position& lastPositionToSelect); > > void insertNodeAfterAndUpdateNodesInserted(PassRefPtr<Node> insertChild, Node* refChild); > - void insertNodeAtAndUpdateNodesInserted(PassRefPtr<Node>, const Position&); > + Node* insertNodeAtAndUpdateNodesInserted(PassRefPtr<Node>, const Position&); > void insertNodeBeforeAndUpdateNodesInserted(PassRefPtr<Node> insertChild, Node* refChild); > + Node* insertAsListItems(PassRefPtr<Node>, Node* insertionNode, const Position&); > > void updateNodesInserted(Node*); > bool shouldRemoveEndBR(Node*, const VisiblePosition&); > diff --git a/WebCore/editing/htmlediting.cpp b/WebCore/editing/htmlediting.cpp > index b58dff3..4981f63 100644 > --- a/WebCore/editing/htmlediting.cpp > +++ b/WebCore/editing/htmlediting.cpp > @@ -658,6 +658,11 @@ bool isListElement(Node *n) > return (n && (n->hasTagName(ulTag) || n->hasTagName(olTag) || n->hasTagName(dlTag))); > } > > +bool isListItem(Node *n) > +{ > + return n && n->renderer() && n->renderer()->isListItem(); > +} > + > Node* enclosingNodeWithTag(const Position& p, const QualifiedName& tagName) > { > if (p.isNull()) > @@ -779,7 +784,7 @@ static Node* appendedSublist(Node* listItem) > for (Node* n = listItem->nextSibling(); n; n = n->nextSibling()) { > if (isListElement(n)) > return static_cast<HTMLElement*>(n); > - if (n->renderer() && n->renderer()->isListItem()) > + if (isListItem(listItem)) > return 0; > } > > diff --git a/WebCore/editing/htmlediting.h b/WebCore/editing/htmlediting.h > index c5a44ac..ef3db35 100644 > --- a/WebCore/editing/htmlediting.h > +++ b/WebCore/editing/htmlediting.h > @@ -90,6 +90,7 @@ bool isTableCell(const Node*); > bool isEmptyTableCell(const Node*); > bool isTableStructureNode(const Node*); > bool isListElement(Node*); > +bool isListItem(Node*); > bool isNodeRendered(const Node*); > bool isNodeVisiblyContainedWithin(Node*, const Range*); > bool isRenderedAsNonInlineTableImageOrHR(const Node*); The approach looks ok to me. I don't like very much the fact that you've changed insertNodeAndUpdateNodesInserted to add a return value that is used only at the beginning of the loop. I'd rather see that method stay the same and add another for the specific purpose. I believe it makes it more clear, it is already such a complex piece of code :-).

Tony Chang

Comment 4 2010-02-03 17:36:14 PST

Created attachment 48083 [details] Patch

Tony Chang

Comment 5 2010-02-03 17:38:12 PST

(In reply to comment #3) > The approach looks ok to me. I don't like very much the fact that you've > changed insertNodeAndUpdateNodesInserted to add a return value that is used > only at the beginning of the loop. > I'd rather see that method stay the same and add another for the specific > purpose. I believe it makes it more clear, it is already such a complex piece > of code :-). Good idea. I restored insertNodeAtAndUpdateNodesInserted and moved the list handling into doApply().

Enrica Casucci

Comment 6 2010-02-03 19:26:11 PST

Comment on attachment 48083 [details] Patch > diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog > index a660e4b..a6c6fe6 100644 > --- a/LayoutTests/ChangeLog > +++ b/LayoutTests/ChangeLog > @@ -1,3 +1,14 @@ > +2010-02-03 Tony Chang <tony@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + https://bugs.webkit.org/show_bug.cgi?id=24872 > + Add a test to make sure copying from a list and pasting into a list > + keeps the list at the same indention level rather than nesting. > + > + * editing/pasteboard/paste-list-002-expected.txt: Added. > + * editing/pasteboard/paste-list-002.html: Added. > + > 2010-02-03 Csaba OsztrogonÃ¡c <ossy@webkit.org> > > Rubber-stamped by Ariya Hidayat. > diff --git a/LayoutTests/editing/pasteboard/paste-list-002-expected.txt b/LayoutTests/editing/pasteboard/paste-list-002-expected.txt > new file mode 100644 > index 0000000..5ff1cb6 > --- /dev/null > +++ b/LayoutTests/editing/pasteboard/paste-list-002-expected.txt > @@ -0,0 +1,13 @@ > +Copy/pasting list items in a list. This test should be run with DRT for pasteboard access. > + > +PASS: <li>alpha</li><ul><li>beta</li><li>gamma</li></ul><li>beta</li><li>gamma</li><li id="delta">delta</li><li>beta</li><li>gamma</li><li></li> > + > +alpha > +beta > +gamma > +beta > +gamma > +delta > +beta > +gamma > + > diff --git a/LayoutTests/editing/pasteboard/paste-list-002.html b/LayoutTests/editing/pasteboard/paste-list-002.html > new file mode 100644 > index 0000000..ff4c97c > --- /dev/null > +++ b/LayoutTests/editing/pasteboard/paste-list-002.html > @@ -0,0 +1,63 @@ > +<html> > +<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> > +<script> > + > +function editingTest() { > + // Select "beta" and "gamma". > + moveSelectionForwardByLineCommand(); > + for (i = 0; i < 2; i++) > + extendSelectionForwardByLineCommand(); > + copyCommand(); > + > + // Paste with the cursor right before "delta". > + moveSelectionForwardByLineCommand(); > + pasteCommand(); > + > + // Verify that the cursor is in the right place (still before delta). > + var selection = window.getSelection(); > + if (selection.baseNode.parentNode != document.getElementById("delta") || > + selection.baseOffset != 0 || !selection.isCollapsed) > + throw "Wrong selection position on before paste."; > + > + // Paste with the cursor at the end of "delta". > + moveSelectionForwardByWordCommand(); > + pasteCommand(); > + > + // Verify that the cursor is in the right place (new list item after delta). > + var selection = window.getSelection(); > + if (!selection.isCollapsed || selection.baseNode.value != "") > + throw "Wrong selection position on after paste."; > +} > + > +</script> > +<body> > +<div contentEditable="true"> > +<p>Copy/pasting list items in a list. This test should be run with DRT for pasteboard access.</p> > +<p id="results">FAIL</p> > +<ul id="test"> > + <li>alpha</li> > + <li>beta</li> > + <li>gamma</li> > + <li id="delta">delta</li> > +</ul> > +</div> > + > +<script> > +if (window.layoutTestController) > + layoutTestController.dumpAsText(); > + > +var elem = document.getElementById("test"); > +var selection = window.getSelection(); > +selection.setPosition(elem, 0); > +editingTest(); > + > +// Rerun the test but have the source list be indented. > +document.getElementById("test").innerHTML = "<li>alpha</li><ul><li>beta</li><li>gamma</li></ul><li id='delta'>delta</li>"; > +selection.setPosition(elem, 0); > +editingTest(); > + > +document.getElementById("results").innerText = "PASS: " + document.getElementById("test").innerHTML; > +</script> > + > +</body> > +</html> > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > index dc9a30a..0e88eb1 100644 > --- a/WebCore/ChangeLog > +++ b/WebCore/ChangeLog > @@ -1,3 +1,29 @@ > +2010-02-03 Tony Chang <tony@chromium.org> > + > + Reviewed by NOBODY (OOPS!). > + > + https://bugs.webkit.org/show_bug.cgi?id=24872 > + When pasting a list into another list should not indent another level. > + If the cursor is at the beginning of the line, it should insert the > + list items before the current list item. If the cursor is at the end > + of the line, it should insert the list items after the current list item. > + > + This matches Firefox and IE and makes the common activity of reordering a list > + work as expected. > + > + This also adds a small helper method (isListItem) to htmlediting.h. > + > + Test: editing/pasteboard/paste-list-002.html > + > + * editing/ReplaceSelectionCommand.cpp: > + (WebCore::ReplaceSelectionCommand::doApply): > + (WebCore::ReplaceSelectionCommand::insertAsListItems): > + * editing/ReplaceSelectionCommand.h: > + * editing/htmlediting.cpp: > + (WebCore::isListItem): > + (WebCore::appendedSublist): > + * editing/htmlediting.h: > + > 2010-02-03 Adele Peterson <adele@apple.com> > > Reviewed by Brady Eidson. > diff --git a/WebCore/editing/ReplaceSelectionCommand.cpp b/WebCore/editing/ReplaceSelectionCommand.cpp > index 85a4471..f26757e 100644 > --- a/WebCore/editing/ReplaceSelectionCommand.cpp > +++ b/WebCore/editing/ReplaceSelectionCommand.cpp > @@ -752,9 +752,7 @@ void ReplaceSelectionCommand::doApply() > bool startIsInsideMailBlockquote = nearestMailBlockquote(insertionPos.node()); > > if ((selectionStartWasStartOfParagraph && selectionEndWasEndOfParagraph && !startIsInsideMailBlockquote) || > - startBlock == currentRoot || > - (startBlock && startBlock->renderer() && startBlock->renderer()->isListItem()) || > - selectionIsPlainText) > + startBlock == currentRoot || isListItem(startBlock) || selectionIsPlainText) > m_preventNesting = false; > > if (selection.isRange()) { > @@ -871,7 +869,12 @@ void ReplaceSelectionCommand::doApply() > RefPtr<Node> node = refNode->nextSibling(); > > fragment.removeNode(refNode); > - insertNodeAtAndUpdateNodesInserted(refNode, insertionPos); > + > + Node* blockStart = enclosingBlock(insertionPos.node()); > + if (isListElement(refNode.get()) && blockStart->renderer()->isListItem()) > + refNode = insertAsListItems(refNode, blockStart, insertionPos); > + else > + insertNodeAtAndUpdateNodesInserted(refNode, insertionPos); > > // Mutation events (bug 22634) may have already removed the inserted content > if (!refNode->inDocument()) > @@ -960,9 +963,15 @@ void ReplaceSelectionCommand::doApply() > if (selectionEndWasEndOfParagraph || !isEndOfParagraph(endOfInsertedContent) || next.isNull()) { > if (!isStartOfParagraph(endOfInsertedContent)) { > setEndingSelection(endOfInsertedContent); > - // Use a default paragraph element (a plain div) for the empty paragraph, using the last paragraph > - // block's style seems to annoy users. > - insertParagraphSeparator(true); > + Node* enclosingNode = enclosingBlock(endOfInsertedContent.deepEquivalent().node()); > + if (isListItem(enclosingNode)) { > + RefPtr<Node> newListItem = createListItemElement(document()); > + insertNodeAfter(newListItem, enclosingNode); > + setEndingSelection(VisiblePosition(Position(newListItem, 0))); > + } else > + // Use a default paragraph element (a plain div) for the empty paragraph, using the last paragraph > + // block's style seems to annoy users. > + insertParagraphSeparator(true); > > // Select up to the paragraph separator that was added. > lastPositionToSelect = endingSelection().visibleStart().deepEquivalent(); > @@ -1111,6 +1120,39 @@ void ReplaceSelectionCommand::insertNodeBeforeAndUpdateNodesInserted(PassRefPtr< > updateNodesInserted(nodeToUpdate); > } > > +// If the user is inserting a list into an existing list, instead of nesting the list, > +// we put the list items into the existing list. > +Node* ReplaceSelectionCommand::insertAsListItems(PassRefPtr<Node> listElement, Node* insertionNode, const Position& p) > +{ > + while (listElement->hasChildNodes() && isListElement(listElement->firstChild()) && listElement->childNodeCount() == 1) > + listElement = listElement->firstChild(); > + > + bool isStart = isStartOfParagraph(p); > + bool isEnd = isEndOfParagraph(p); > + > + Node* lastNode = insertionNode; > + while (RefPtr<Node> listItem = listElement->firstChild()) { > + ExceptionCode ec = 0; > + listElement->removeChild(listItem.get(), ec); > + ASSERT(!ec); > + if (isStart) > + insertNodeBefore(listItem, lastNode); > + else if (isEnd) { > + insertNodeAfter(listItem, lastNode); > + lastNode = listItem.get(); > + } else { > + // FIXME: If we're in the middle of a list item, we should split it into two separate > + // list items and insert these nodes between them. For now, just append the nodes. > + insertNodeAfter(listItem, lastNode); > + lastNode = listItem.get(); > + } > + } > + if (isStart) > + lastNode = lastNode->previousSibling(); > + updateNodesInserted(lastNode); > + return lastNode; > +} > + > void ReplaceSelectionCommand::updateNodesInserted(Node *node) > { > if (!node) > diff --git a/WebCore/editing/ReplaceSelectionCommand.h b/WebCore/editing/ReplaceSelectionCommand.h > index 1cb93c3..19f63bb 100644 > --- a/WebCore/editing/ReplaceSelectionCommand.h > +++ b/WebCore/editing/ReplaceSelectionCommand.h > @@ -54,6 +54,7 @@ private: > void insertNodeAfterAndUpdateNodesInserted(PassRefPtr<Node> insertChild, Node* refChild); > void insertNodeAtAndUpdateNodesInserted(PassRefPtr<Node>, const Position&); > void insertNodeBeforeAndUpdateNodesInserted(PassRefPtr<Node> insertChild, Node* refChild); > + Node* insertAsListItems(PassRefPtr<Node>, Node* insertionNode, const Position&); > > void updateNodesInserted(Node*); > bool shouldRemoveEndBR(Node*, const VisiblePosition&); > diff --git a/WebCore/editing/htmlediting.cpp b/WebCore/editing/htmlediting.cpp > index b58dff3..4981f63 100644 > --- a/WebCore/editing/htmlediting.cpp > +++ b/WebCore/editing/htmlediting.cpp > @@ -658,6 +658,11 @@ bool isListElement(Node *n) > return (n && (n->hasTagName(ulTag) || n->hasTagName(olTag) || n->hasTagName(dlTag))); > } > > +bool isListItem(Node *n) > +{ > + return n && n->renderer() && n->renderer()->isListItem(); > +} > + > Node* enclosingNodeWithTag(const Position& p, const QualifiedName& tagName) > { > if (p.isNull()) > @@ -779,7 +784,7 @@ static Node* appendedSublist(Node* listItem) > for (Node* n = listItem->nextSibling(); n; n = n->nextSibling()) { > if (isListElement(n)) > return static_cast<HTMLElement*>(n); > - if (n->renderer() && n->renderer()->isListItem()) > + if (isListItem(listItem)) > return 0; > } > > diff --git a/WebCore/editing/htmlediting.h b/WebCore/editing/htmlediting.h > index c5a44ac..ef3db35 100644 > --- a/WebCore/editing/htmlediting.h > +++ b/WebCore/editing/htmlediting.h > @@ -90,6 +90,7 @@ bool isTableCell(const Node*); > bool isEmptyTableCell(const Node*); > bool isTableStructureNode(const Node*); > bool isListElement(Node*); > +bool isListItem(Node*); > bool isNodeRendered(const Node*); > bool isNodeVisiblyContainedWithin(Node*, const Range*); > bool isRenderedAsNonInlineTableImageOrHR(const Node*); It looks good to me.

Eric Seidel (no email)

Comment 7 2010-02-04 16:26:34 PST

Comment on attachment 48083 [details] Patch The language/type stuff is unneeded: 2 <script src=../editing.js language="JavaScript" type="text/JavaScript" ></script> Looks sane.

Tony Chang

Comment 8 2010-02-05 01:23:21 PST

Committed r54413: <http://trac.webkit.org/changeset/54413>

Attachments
Patch (11.87 KB, patch) 2010-02-02 21:15 PST, Tony Chang	no flags	Details Formatted Diff Diff
Patch (10.98 KB, patch) 2010-02-03 17:36 PST, Tony Chang	eric: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Bug 24872