WebCore:
authorjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2007 21:32:12 +0000 (21:32 +0000)
committerjustin.garcia@apple.com <justin.garcia@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Dec 2007 21:32:12 +0000 (21:32 +0000)
        Reviewed by Darin Adler.

        <rdar://problem/5601583> GMail Editor: Copied link doesn't paste as a link, just colored text

        The code that checks the selected Range to see if it's inside an anchor
        checks ancestors of the Range's commonAncestor() but not the
        commonAncestor() itself, and so we'd fail to add markup for the enclosing
        anchor to the pasteboard.

        Some enclosing element getters check the node passed to the getter and some
        don't.  There were a few places where we incorrectly assumed that enclosing
        element getters check the node passed to the getter, but this is the only
        case that I'm able to write a test case for at the moment.

        In this patch I've:
        Changed enclosingNodeWithType and enclosingNodeWithTag to take in positions,
        like the newer enclosing element getters.  This is important because we must
        soon add code to the getters so that they understand that some editing positions
        inside nodes don't actually refer to positions inside those nodes but positions
        before and after them.  Like [table, 0].
        Changed enclosingNodeWithType and enclosingNodeWithTag to check nodes starting with
        n where [n, o] is the position passed to the getter, instead of starting the the parent
        of n.  This makes all but a few of the enclosing element getters behave consistently.
        Changed enclosingNodeWithType and enclosingNodeWithTag to not return non-editable
        nodes if the input position was editable.  This fixes a bug that that the above change
        exposed.
        Changed enclosingTableCell to simply call enclosingNodeWithType.  We should do
        this for the rest of the getters, or simply remove them in favor of enclosingNodeWithType
        unless doing so would affect readability, like it would in the case of enclosingTableCell.
        Ditto for enclosingBlock.

        * editing/AppendNodeCommand.cpp:
        (WebCore::AppendNodeCommand::doApply):
        * editing/DeleteButtonController.cpp:
        (WebCore::enclosingDeletableElement):
        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::initializePositionData):
        (WebCore::DeleteSelectionCommand::saveFullySelectedAnchor):
        * editing/Editor.cpp:
        (WebCore::Editor::selectionUnorderedListState):
        (WebCore::Editor::selectionOrderedListState):
        * editing/IndentOutdentCommand.cpp:
        (WebCore::IndentOutdentCommand::prepareBlockquoteLevelForInsertion):
        (WebCore::IndentOutdentCommand::outdentParagraph):
        * editing/InsertNodeBeforeCommand.cpp:
        (WebCore::InsertNodeBeforeCommand::doApply):
        * editing/InsertParagraphSeparatorCommand.cpp:
        (WebCore::InsertParagraphSeparatorCommand::doApply):
        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::shouldMerge):
        (WebCore::ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds):
        (WebCore::ReplaceSelectionCommand::positionAtEndOfInsertedContent):
        * editing/TextIterator.cpp:
        * editing/htmlediting.cpp:
        (WebCore::enclosingBlock):
        (WebCore::enclosingNodeWithTag):
        (WebCore::enclosingNodeOfType):
        (WebCore::enclosingTableCell):
        (WebCore::isTableCell):
        * editing/htmlediting.h:
        * editing/markup.cpp:
        (WebCore::appendStartMarkup):
        (WebCore::createMarkup):

LayoutTests:

        Reviewed by Darin Adler.

        <rdar://problem/5601583> GMail Editor: Copied link doesn't paste as a link, just colored text

        * editing/pasteboard/5601583-1.html: Added.
        * platform/mac/editing/pasteboard/5601583-1-expected.checksum: Added.
        * platform/mac/editing/pasteboard/5601583-1-expected.png: Added.
        * platform/mac/editing/pasteboard/5601583-1-expected.txt: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@28694 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/5601583-1.html [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/AppendNodeCommand.cpp
WebCore/editing/DeleteButtonController.cpp
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/Editor.cpp
WebCore/editing/IndentOutdentCommand.cpp
WebCore/editing/InsertNodeBeforeCommand.cpp
WebCore/editing/InsertParagraphSeparatorCommand.cpp
WebCore/editing/ReplaceSelectionCommand.cpp
WebCore/editing/TextIterator.cpp
WebCore/editing/htmlediting.cpp
WebCore/editing/htmlediting.h
WebCore/editing/markup.cpp

index 7132daa..e4c93fa 100644 (file)
@@ -1,3 +1,14 @@
+2007-12-13  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <rdar://problem/5601583> GMail Editor: Copied link doesn't paste as a link, just colored text
+
+        * editing/pasteboard/5601583-1.html: Added.
+        * platform/mac/editing/pasteboard/5601583-1-expected.checksum: Added.
+        * platform/mac/editing/pasteboard/5601583-1-expected.png: Added.
+        * platform/mac/editing/pasteboard/5601583-1-expected.txt: Added.
+
 2007-12-13  Kevin McCullough  <kmccullough@apple.com>
 
         Reviewed by Alice and Sam.
diff --git a/LayoutTests/editing/pasteboard/5601583-1.html b/LayoutTests/editing/pasteboard/5601583-1.html
new file mode 100644 (file)
index 0000000..6532d9e
--- /dev/null
@@ -0,0 +1,16 @@
+<p>This tests for a bug where a copied link wouldn't paste as a link.  Both editable regions below should contain a link "Hello\nWorld".</p>
+<div id="copy" contenteditable="true">
+<a href="http://www.apple.com/">Hello<br>World</a>
+</div>
+
+<div id="paste" contenteditable="true"><br></div>
+
+<script>
+copy = document.getElementById("copy");
+copy.focus();
+document.execCommand("Copy");
+
+paste = document.getElementById("paste");
+paste.focus();
+document.execCommand("Paste");
+</script>
diff --git a/LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.checksum b/LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.checksum
new file mode 100644 (file)
index 0000000..34d9e90
--- /dev/null
@@ -0,0 +1 @@
+270302b8eddec7f8223623e1cdb56566
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.png b/LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.png
new file mode 100644 (file)
index 0000000..2711d89
Binary files /dev/null and b/LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.txt b/LayoutTests/platform/mac/editing/pasteboard/5601583-1-expected.txt
new file mode 100644 (file)
index 0000000..fc8b312
--- /dev/null
@@ -0,0 +1,26 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 719x36
+          text run at (0,0) width 407: "This tests for a bug where a copied link wouldn't paste as a link. "
+          text run at (407,0) width 312: "Both editable regions below should contain a link"
+          text run at (0,18) width 105: "\"Hello\\nWorld\"."
+      RenderBlock {DIV} at (0,52) size 784x36
+        RenderInline {A} at (0,0) size 40x36 [color=#0000EE]
+          RenderText {#text} at (0,0) size 35x18
+            text run at (0,0) width 35: "Hello"
+          RenderBR {BR} at (35,14) size 0x0
+          RenderText {#text} at (0,18) size 40x18
+            text run at (0,18) width 40: "World"
+        RenderText {#text} at (0,0) size 0x0
+      RenderBlock {DIV} at (0,88) size 784x36
+        RenderInline {A} at (0,0) size 40x36 [color=#0000EE]
+          RenderText {#text} at (0,0) size 35x18
+            text run at (0,0) width 35: "Hello"
+          RenderBR {BR} at (35,14) size 0x0
+          RenderText {#text} at (0,18) size 40x18
+            text run at (0,18) width 40: "World"
+caret: position 5 of child 2 {#text} of child 0 {A} of child 4 {DIV} of child 0 {BODY} of child 0 {HTML} of document
index 1bec79d..4dddab6 100644 (file)
@@ -1,3 +1,69 @@
+2007-12-13  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin Adler.
+
+        <rdar://problem/5601583> GMail Editor: Copied link doesn't paste as a link, just colored text
+        
+        The code that checks the selected Range to see if it's inside an anchor
+        checks ancestors of the Range's commonAncestor() but not the
+        commonAncestor() itself, and so we'd fail to add markup for the enclosing
+        anchor to the pasteboard.
+        
+        Some enclosing element getters check the node passed to the getter and some
+        don't.  There were a few places where we incorrectly assumed that enclosing 
+        element getters check the node passed to the getter, but this is the only 
+        case that I'm able to write a test case for at the moment.
+        
+        In this patch I've:
+        Changed enclosingNodeWithType and enclosingNodeWithTag to take in positions,
+        like the newer enclosing element getters.  This is important because we must
+        soon add code to the getters so that they understand that some editing positions
+        inside nodes don't actually refer to positions inside those nodes but positions
+        before and after them.  Like [table, 0].
+        Changed enclosingNodeWithType and enclosingNodeWithTag to check nodes starting with
+        n where [n, o] is the position passed to the getter, instead of starting the the parent
+        of n.  This makes all but a few of the enclosing element getters behave consistently.
+        Changed enclosingNodeWithType and enclosingNodeWithTag to not return non-editable 
+        nodes if the input position was editable.  This fixes a bug that that the above change
+        exposed.
+        Changed enclosingTableCell to simply call enclosingNodeWithType.  We should do
+        this for the rest of the getters, or simply remove them in favor of enclosingNodeWithType
+        unless doing so would affect readability, like it would in the case of enclosingTableCell.
+        Ditto for enclosingBlock.
+
+        * editing/AppendNodeCommand.cpp:
+        (WebCore::AppendNodeCommand::doApply):
+        * editing/DeleteButtonController.cpp:
+        (WebCore::enclosingDeletableElement):
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializePositionData):
+        (WebCore::DeleteSelectionCommand::saveFullySelectedAnchor):
+        * editing/Editor.cpp:
+        (WebCore::Editor::selectionUnorderedListState):
+        (WebCore::Editor::selectionOrderedListState):
+        * editing/IndentOutdentCommand.cpp:
+        (WebCore::IndentOutdentCommand::prepareBlockquoteLevelForInsertion):
+        (WebCore::IndentOutdentCommand::outdentParagraph):
+        * editing/InsertNodeBeforeCommand.cpp:
+        (WebCore::InsertNodeBeforeCommand::doApply):
+        * editing/InsertParagraphSeparatorCommand.cpp:
+        (WebCore::InsertParagraphSeparatorCommand::doApply):
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::shouldMerge):
+        (WebCore::ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds):
+        (WebCore::ReplaceSelectionCommand::positionAtEndOfInsertedContent):
+        * editing/TextIterator.cpp:
+        * editing/htmlediting.cpp:
+        (WebCore::enclosingBlock):
+        (WebCore::enclosingNodeWithTag):
+        (WebCore::enclosingNodeOfType):
+        (WebCore::enclosingTableCell):
+        (WebCore::isTableCell):
+        * editing/htmlediting.h:
+        * editing/markup.cpp:
+        (WebCore::appendStartMarkup):
+        (WebCore::createMarkup):
+
 2007-12-13  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Darin.
index 4a392dc..ab68d6b 100644 (file)
@@ -43,7 +43,7 @@ void AppendNodeCommand::doApply()
     // If the child to append is already in a tree, appending it will remove it from it's old location
     // in an non-undoable way.  We might eventually find it useful to do an undoable remove in this case.
     ASSERT(!m_childToAppend->parent());
-    ASSERT(isContentEditable(m_parentNode.get()) || enclosingNodeOfType(m_parentNode.get(), &isContentEditable) || !m_parentNode->attached());
+    ASSERT(enclosingNodeOfType(Position(m_parentNode.get(), 0), &isContentEditable) || !m_parentNode->attached());
 
     ExceptionCode ec = 0;
     m_parentNode->appendChild(m_childToAppend.get(), ec);
index e41d4c5..6627f00 100644 (file)
@@ -115,13 +115,7 @@ static HTMLElement* enclosingDeletableElement(const Selection& selection)
     if (!container->isContentEditable())
         return 0;
 
-    // enclosingNodeOfType only looks at ancestor nodes, we also consider the container for deletion
-    if (isDeletableElement(container)) {
-        ASSERT(container->isHTMLElement());
-        return static_cast<HTMLElement*>(container);
-    }
-
-    Node* element = enclosingNodeOfType(container, &isDeletableElement);
+    Node* element = enclosingNodeOfType(Position(container, 0), &isDeletableElement);
     if (!element)
         return 0;
 
index 463d80c..6bedd69 100644 (file)
@@ -47,11 +47,6 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-static bool isTableCell(Node* node)
-{
-    return node && (node->hasTagName(tdTag) || node->hasTagName(thTag));
-}
-
 static bool isTableRow(const Node* node)
 {
     return node && node->hasTagName(trTag);
@@ -171,8 +166,8 @@ void DeleteSelectionCommand::initializePositionData()
     m_startRoot = editableRootForPosition(start);
     m_endRoot = editableRootForPosition(end);
     
-    m_startTableRow = enclosingNodeOfType(start.node(), &isTableRow);
-    m_endTableRow = enclosingNodeOfType(end.node(), &isTableRow);
+    m_startTableRow = enclosingNodeOfType(start, &isTableRow);
+    m_endTableRow = enclosingNodeOfType(end, &isTableRow);
     
     Node* startCell = enclosingTableCell(m_upstreamStart);
     Node* endCell = enclosingTableCell(m_downstreamEnd);
@@ -657,11 +652,11 @@ void DeleteSelectionCommand::saveFullySelectedAnchor()
     // when the user begins entering text.
     VisiblePosition visibleStart = m_selectionToDelete.visibleStart();
     VisiblePosition visibleEnd = m_selectionToDelete.visibleEnd();
-    Node* startAnchor = enclosingNodeWithTag(visibleStart.deepEquivalent().downstream().node(), aTag);
-    Node* endAnchor = enclosingNodeWithTag(visibleEnd.deepEquivalent().upstream().node(), aTag);
+    Node* startAnchor = enclosingNodeWithTag(visibleStart.deepEquivalent().downstream(), aTag);
+    Node* endAnchor = enclosingNodeWithTag(visibleEnd.deepEquivalent().upstream(), aTag);
 
-    Node* beforeStartAnchor = enclosingNodeWithTag(visibleStart.previous().deepEquivalent().downstream().node(), aTag);
-    Node* afterEndAnchor = enclosingNodeWithTag(visibleEnd.next().deepEquivalent().upstream().node(), aTag);
+    Node* beforeStartAnchor = enclosingNodeWithTag(visibleStart.previous().deepEquivalent().downstream(), aTag);
+    Node* afterEndAnchor = enclosingNodeWithTag(visibleEnd.next().deepEquivalent().upstream(), aTag);
 
     if (startAnchor && startAnchor == endAnchor && startAnchor != beforeStartAnchor && endAnchor != afterEndAnchor)
         document()->frame()->editor()->setRemovedAnchor(startAnchor->cloneNode(false));
index 6c5ba30..5ed6012 100644 (file)
@@ -461,12 +461,11 @@ const FontData* Editor::fontForSelection(bool& hasMultipleFonts) const
 TriState Editor::selectionUnorderedListState() const
 {
     if (m_frame->selectionController()->isCaret()) {
-        Node* selectionNode = m_frame->selectionController()->selection().start().node();
-        if (enclosingNodeWithTag(selectionNode, ulTag))
+        if (enclosingNodeWithTag(m_frame->selectionController()->selection().start(), ulTag))
             return TrueTriState;
     } else if (m_frame->selectionController()->isRange()) {
-        Node* startNode = enclosingNodeWithTag(m_frame->selectionController()->selection().start().node(), ulTag);
-        Node* endNode = enclosingNodeWithTag(m_frame->selectionController()->selection().end().node(), ulTag);
+        Node* startNode = enclosingNodeWithTag(m_frame->selectionController()->selection().start(), ulTag);
+        Node* endNode = enclosingNodeWithTag(m_frame->selectionController()->selection().end(), ulTag);
         if (startNode && endNode && startNode == endNode)
             return TrueTriState;
     }
@@ -477,12 +476,11 @@ TriState Editor::selectionUnorderedListState() const
 TriState Editor::selectionOrderedListState() const
 {
     if (m_frame->selectionController()->isCaret()) {
-        Node* selectionNode = m_frame->selectionController()->selection().start().node();
-        if (enclosingNodeWithTag(selectionNode, olTag))
+        if (enclosingNodeWithTag(m_frame->selectionController()->selection().start(), olTag))
             return TrueTriState;
     } else if (m_frame->selectionController()->isRange()) {
-        Node* startNode = enclosingNodeWithTag(m_frame->selectionController()->selection().start().node(), olTag);
-        Node* endNode = enclosingNodeWithTag(m_frame->selectionController()->selection().end().node(), olTag);
+        Node* startNode = enclosingNodeWithTag(m_frame->selectionController()->selection().start(), olTag);
+        Node* endNode = enclosingNodeWithTag(m_frame->selectionController()->selection().end(), olTag);
         if (startNode && endNode && startNode == endNode)
             return TrueTriState;
     }
index fbbf036..ce0620e 100644 (file)
@@ -80,10 +80,10 @@ Node* IndentOutdentCommand::prepareBlockquoteLevelForInsertion(VisiblePosition&
     int currentBlockquoteLevel = 0;
     int lastBlockquoteLevel = 0;
     Node* node = currentParagraph.deepEquivalent().node();
-    while ((node = enclosingNodeOfType(node, &isIndentBlockquote)))
+    while ((node = enclosingNodeOfType(Position(node->parentNode(), 0), &isIndentBlockquote)))
         currentBlockquoteLevel++;
     node = *lastBlockquote;
-    while ((node = enclosingNodeOfType(node, &isIndentBlockquote)))
+    while ((node = enclosingNodeOfType(Position(node->parentNode(), 0), &isIndentBlockquote)))
         lastBlockquoteLevel++;
     while (currentBlockquoteLevel > lastBlockquoteLevel) {
         RefPtr<Node> newBlockquote = createIndentBlockquoteElement(document());
@@ -92,7 +92,7 @@ Node* IndentOutdentCommand::prepareBlockquoteLevelForInsertion(VisiblePosition&
         lastBlockquoteLevel++;
     }
     while (currentBlockquoteLevel < lastBlockquoteLevel) {
-        *lastBlockquote = enclosingNodeOfType(*lastBlockquote, &isIndentBlockquote);
+        *lastBlockquote = enclosingNodeOfType(Position((*lastBlockquote)->parentNode(), 0), &isIndentBlockquote);
         lastBlockquoteLevel--;
     }
     RefPtr<Node> placeholder = createBreakElement(document());
@@ -207,7 +207,7 @@ void IndentOutdentCommand::outdentParagraph()
     VisiblePosition visibleStartOfParagraph = startOfParagraph(endingSelection().visibleStart());
     VisiblePosition visibleEndOfParagraph = endOfParagraph(visibleStartOfParagraph);
 
-    Node* enclosingNode = enclosingNodeOfType(visibleStartOfParagraph.deepEquivalent().node(), &isListOrIndentBlockquote);
+    Node* enclosingNode = enclosingNodeOfType(visibleStartOfParagraph.deepEquivalent(), &isListOrIndentBlockquote);
     if (!enclosingNode)
         return;
 
index 6b6aede..ddc4768 100644 (file)
@@ -44,7 +44,7 @@ void InsertNodeBeforeCommand::doApply()
     // If the child to insert is already in a tree, inserting it will remove it from it's old location
     // in an non-undoable way.  We might eventually find it useful to do an undoable remove in this case.
     ASSERT(!m_insertChild->parent());
-    ASSERT(enclosingNodeOfType(m_refChild.get(), &isContentEditable) || !m_refChild->parentNode()->attached());
+    ASSERT(enclosingNodeOfType(Position(m_refChild->parentNode(), 0), &isContentEditable) || !m_refChild->parentNode()->attached());
 
     ExceptionCode ec = 0;
     m_refChild->parentNode()->insertBefore(m_insertChild.get(), m_refChild.get(), ec);
index 2543149..4c5033b 100644 (file)
@@ -101,7 +101,7 @@ void InsertParagraphSeparatorCommand::doApply()
     Node* block = enclosingBlock(pos.node());
     Position canonicalPos = VisiblePosition(pos).deepEquivalent();
     if (!block || !block->parentNode() || 
-        block->renderer() && block->renderer()->isTableCell() ||
+        isTableCell(block) ||
         canonicalPos.node()->renderer() && canonicalPos.node()->renderer()->isTable() ||
         canonicalPos.node()->hasTagName(hrTag)) {
         applyCommandToComposite(new InsertLineBreakCommand(document()));
index 309eaa1..a110480 100644 (file)
@@ -358,7 +358,7 @@ bool ReplaceSelectionCommand::shouldMerge(const VisiblePosition& from, const Vis
     Node* fromNode = from.deepEquivalent().node();
     Node* toNode = to.deepEquivalent().node();
     Node* fromNodeBlock = enclosingBlock(fromNode);
-    return !enclosingNodeOfType(fromNode, &isMailPasteAsQuotationNode) &&
+    return !enclosingNodeOfType(from.deepEquivalent(), &isMailPasteAsQuotationNode) &&
            fromNodeBlock && (!fromNodeBlock->hasTagName(blockquoteTag) || isMailBlockquote(fromNodeBlock))  &&
            enclosingListChild(fromNode) == enclosingListChild(toNode) &&
            enclosingTableCell(from.deepEquivalent()) == enclosingTableCell(from.deepEquivalent()) &&
@@ -396,8 +396,8 @@ void ReplaceSelectionCommand::removeUnrenderedTextNodesAtEnds()
     document()->updateLayoutIgnorePendingStylesheets();
     if (!m_lastLeafInserted->renderer() && 
         m_lastLeafInserted->isTextNode() && 
-        !enclosingNodeWithTag(m_lastLeafInserted.get(), selectTag) && 
-        !enclosingNodeWithTag(m_lastLeafInserted.get(), scriptTag)) {
+        !enclosingNodeWithTag(Position(m_lastLeafInserted.get(), 0), selectTag) && 
+        !enclosingNodeWithTag(Position(m_lastLeafInserted.get(), 0), scriptTag)) {
         RefPtr<Node> previous = m_firstNodeInserted == m_lastLeafInserted ? 0 : m_lastLeafInserted->traversePreviousNode();
         removeNode(m_lastLeafInserted.get());
         m_lastLeafInserted = previous;
@@ -496,7 +496,7 @@ void ReplaceSelectionCommand::handlePasteAsQuotationNode()
 VisiblePosition ReplaceSelectionCommand::positionAtEndOfInsertedContent()
 {
     Node* lastNode = m_lastLeafInserted.get();
-    Node* enclosingSelect = enclosingNodeWithTag(lastNode, selectTag);
+    Node* enclosingSelect = enclosingNodeWithTag(Position(lastNode, 0), selectTag);
     if (enclosingSelect)
         lastNode = enclosingSelect;
     return VisiblePosition(Position(lastNode, maxDeepOffset(lastNode)));
index 0765238..9a10f85 100644 (file)
@@ -374,15 +374,6 @@ bool TextIterator::handleReplacedElement()
     return true;
 }
 
-static bool isTableCell(Node* node)
-{
-    RenderObject* r = node->renderer();
-    if (!r)
-        return node->hasTagName(tdTag) || node->hasTagName(thTag);
-    
-    return r->isTableCell();
-}
-
 static bool shouldEmitTabBeforeNode(Node* node)
 {
     RenderObject* r = node->renderer();
index d2636d0..8d3d134 100644 (file)
@@ -296,10 +296,7 @@ bool isBlock(const Node* node)
 // knowing about these kinds of special cases.
 Node* enclosingBlock(Node* node)
 {
-    if (isBlock(node))
-        return node;
-        
-    return enclosingNodeOfType(node, &isBlock);
+    return enclosingNodeOfType(Position(node, 0), &isBlock);
 }
 
 Position rangeCompliantEquivalent(const Position& pos)
@@ -559,14 +556,15 @@ bool isListElement(Node *n)
     return (n && (n->hasTagName(ulTag) || n->hasTagName(olTag) || n->hasTagName(dlTag)));
 }
 
-Node* enclosingNodeWithTag(Node* node, const QualifiedName& tagName)
+Node* enclosingNodeWithTag(const Position& p, const QualifiedName& tagName)
 {
-    if (!node)
+    if (p.isNull())
         return 0;
         
-    Node* root = highestEditableRoot(Position(node, 0));
-    
-    for (Node* n = node->parentNode(); n; n = n->parentNode()) {
+    Node* root = highestEditableRoot(p);
+    for (Node* n = p.node(); n; n = n->parentNode()) {
+        if (root && !isContentEditable(n))
+            continue;
         if (n->hasTagName(tagName))
             return n;
         if (n == root)
@@ -576,16 +574,17 @@ Node* enclosingNodeWithTag(Node* node, const QualifiedName& tagName)
     return 0;
 }
 
-Node* enclosingNodeOfType(Node* node, bool (*nodeIsOfType)(const Node*))
+Node* enclosingNodeOfType(const Position& p, bool (*nodeIsOfType)(const Node*))
 {
-    if (!node)
+    if (p.isNull())
         return 0;
         
-    Node* root = highestEditableRoot(Position(node, 0));
-    if (root == node)
-        return 0;
-    
-    for (Node* n = node->parentNode(); n; n = n->parentNode()) {
+    Node* root = highestEditableRoot(p);
+    for (Node* n = p.node(); n; n = n->parentNode()) {
+        // Don't return a non-editable node if the input position was editable, since
+        // the callers from editing will no doubt want to perform editing inside the returned node.
+        if (root && !isContentEditable(n))
+            continue;
         if ((*nodeIsOfType)(n))
             return n;
         if (n == root)
@@ -597,16 +596,7 @@ Node* enclosingNodeOfType(Node* node, bool (*nodeIsOfType)(const Node*))
 
 Node* enclosingTableCell(const Position& p)
 {
-    if (p.isNull())
-        return 0;
-    // Note: Should theoretically start with p.node()->parentNode() if p is a position 
-    // that internally means before or after p.node(), but we don't use position's like 
-    // that for table cells.
-    for (Node* n = p.node(); n; n = n->parentNode())
-        if (n->renderer() && n->renderer()->isTableCell())
-            return n;
-            
-    return 0;
+    return enclosingNodeOfType(p, &isTableCell);
 }
 
 Node* enclosingAnchorElement(const Position& p)
@@ -736,6 +726,15 @@ bool isTableElement(Node* n)
     return (renderer && (renderer->style()->display() == TABLE || renderer->style()->display() == INLINE_TABLE));
 }
 
+bool isTableCell(const Node* node)
+{
+    RenderObject* r = node->renderer();
+    if (!r)
+        return node->hasTagName(tdTag) || node->hasTagName(thTag);
+    
+    return r->isTableCell();
+}
+
 PassRefPtr<Element> createDefaultParagraphElement(Document *document)
 {
     ExceptionCode ec = 0;
index 37e7667..4ee6695 100644 (file)
@@ -110,8 +110,8 @@ Position positionOutsideContainingSpecialElement(const Position&, Node** contain
 Node* isLastPositionBeforeTable(const VisiblePosition&);
 Node* isFirstPositionAfterTable(const VisiblePosition&);
 
-Node* enclosingNodeWithTag(Node*, const QualifiedName&);
-Node* enclosingNodeOfType(Node*, bool (*nodeIsOfType)(const Node*));
+Node* enclosingNodeWithTag(const Position&, const QualifiedName&);
+Node* enclosingNodeOfType(const Position&, bool (*nodeIsOfType)(const Node*));
 Node* enclosingTableCell(const Position&);
 Node* enclosingEmptyListItem(const VisiblePosition&);
 Node* enclosingAnchorElement(const Position&);
@@ -121,6 +121,7 @@ Node* outermostEnclosingList(Node*);
 Node* enclosingListChild(Node*);
 Node* highestAncestor(Node*);
 bool isTableElement(Node*);
+bool isTableCell(const Node*);
 
 bool lineBreakExistsAtPosition(const VisiblePosition&);
 
index f7f15b5..5cdeb47 100644 (file)
@@ -364,7 +364,7 @@ static void appendStartMarkup(Vector<UChar>& result, const Node *node, const Ran
                 break;
             }
             
-            bool useRenderedText = !enclosingNodeWithTag(const_cast<Node*>(node), selectTag);
+            bool useRenderedText = !enclosingNodeWithTag(Position(const_cast<Node*>(node), 0), selectTag);
             DeprecatedString markup = escapeContentText(useRenderedText ? renderedText(node, range) : stringValueForRange(node, range));
             if (annotate)
                 markup = convertHTMLTextToInterchangeFormat(markup, static_cast<const Text*>(node));
@@ -689,7 +689,7 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
         bool skipDescendants = false;
         bool addMarkupForNode = true;
         
-        if (!n->renderer() && !enclosingNodeWithTag(n, selectTag)) {
+        if (!n->renderer() && !enclosingNodeWithTag(Position(n, 0), selectTag)) {
             skipDescendants = true;
             addMarkupForNode = false;
             next = n->traverseNextSibling();
@@ -778,13 +778,13 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
     if (checkAncestor->renderer()) {
         RefPtr<CSSMutableStyleDeclaration> checkAncestorStyle = computedStyle(checkAncestor)->copyInheritableProperties();
         if (!propertyMissingOrEqualToNone(checkAncestorStyle.get(), CSS_PROP__WEBKIT_TEXT_DECORATIONS_IN_EFFECT))
-            specialCommonAncestor = elementHasTextDecorationProperty(checkAncestor) ? checkAncestor : enclosingNodeOfType(checkAncestor, &elementHasTextDecorationProperty);
+            specialCommonAncestor = enclosingNodeOfType(Position(checkAncestor, 0), &elementHasTextDecorationProperty);
     }
     
-    if (Node *enclosingAnchor = enclosingNodeWithTag(specialCommonAncestor ? specialCommonAncestor : commonAncestor, aTag))
+    if (Node *enclosingAnchor = enclosingNodeWithTag(Position(specialCommonAncestor ? specialCommonAncestor : commonAncestor, 0), aTag))
         specialCommonAncestor = enclosingAnchor;
     
-    Node* body = enclosingNodeWithTag(commonAncestor, bodyTag);
+    Node* body = enclosingNodeWithTag(Position(commonAncestor, 0), bodyTag);
     // FIXME: Only include markup for a fully selected root (and ancestors of lastClosed up to that root) if
     // there are styles/attributes on those nodes that need to be included to preserve the appearance of the copied markup.
     // FIXME: Do this for all fully selected blocks, not just the body.