Changeset 38683 in webkit


Ignore:
Timestamp:
Nov 21, 2008 5:50:42 PM (15 years ago)
Author:
justin.garcia@apple.com
Message:

WebCore:

2008-11-21 Justin Garcia <​justin.garcia@apple.com>

Reviewed by Darin Adler.

<rdar://problem/5381788> Match NSTextView editing behavior at the end of hyperlink text


Change link editing behavior to match TextEdit and MS Word when editing before and after
a link (Pages has two caret positions at link boundaries, Thunderbird and FF behave like we
used to, so it's difficult to get out of link editing mode):
When inserting before or after a link, always insert content outside of the link. This
makes it impossible to get stuck in link editing mode, while making it slightly more
difficult to edit link labels. WebKit editors that care about this can add UI for editing
link labels, like GMail and GoogleDocs have done. We never actually had any bugs complaining
about how it was difficult to edit link labels at the start/end, the code was just introduced
with another bug fix without much thought.


Don't remember removed links anymore, no other editor does this and it made it
difficult/impossible to get out of link editing mode. This code was added to fix
<rdar://problem/4069359>, which is fixed instead by removing the styles from an
enclosing anchor element from those styles that we remember when we delete content.

  • editing/CompositeEditCommand.cpp: (WebCore::CompositeEditCommand::positionAvoidingSpecialElementBoundary):
  • editing/CompositeEditCommand.h:
  • editing/DeleteSelectionCommand.cpp: (WebCore::removeEnclosingAnchorStyle): (WebCore::DeleteSelectionCommand::saveTypingStyleState): (WebCore::DeleteSelectionCommand::doApply):
  • editing/DeleteSelectionCommand.h:
  • editing/EditCommand.cpp: (WebCore::EditCommand::apply):
  • editing/Editor.cpp: (WebCore::Editor::appliedEditing):
  • editing/InsertTextCommand.cpp: (WebCore::InsertTextCommand::prepareForTextInsertion): (WebCore::InsertTextCommand::input):
  • editing/RemoveFormatCommand.cpp: (WebCore::RemoveFormatCommand::doApply):
  • editing/SelectionController.cpp: (WebCore::SelectionController::setSelection):
  • editing/SelectionController.h:

LayoutTests:

2008-11-21 Justin Garcia <​justin.garcia@apple.com>

Reviewed by Darin Adler.


<rdar://problem/5381788> Match NSTextView editing behavior at the end of hyperlink text


Removed tests for behaviors that we're no longer interested in:

  • editing/deleting/delete-link-1.html: Removed.
  • platform/mac/editing/deleting/delete-link-1-expected.checksum: Removed.
  • platform/mac/editing/deleting/delete-link-1-expected.png: Removed.
  • platform/mac/editing/deleting/delete-link-1-expected.txt: Removed.
  • editing/execCommand/19653-4-expected.txt: Removed.
  • editing/execCommand/19653-4.html: Removed.


Reflects new behavior (don't remember removed anchors):

  • platform/mac/editing/deleting/5168598-expected.txt:


Reflects new behavior. Also made this test cross-platform:

  • editing/inserting/insert-before-link-1-expected.txt: Added.
  • editing/inserting/insert-before-link-1.html:
  • platform/mac/editing/inserting/insert-before-link-1-expected.checksum: Removed.
  • platform/mac/editing/inserting/insert-before-link-1-expected.png: Removed.
  • platform/mac/editing/inserting/insert-before-link-1-expected.txt: Removed.
Location:
trunk
Files:
1 added
9 deleted
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r38678 r38683  
     12008-11-21  Justin Garcia  <justin.garcia@apple.com>
     2
     3        Reviewed by Darin Adler.
     4       
     5        <rdar://problem/5381788> Match NSTextView editing behavior at the end of hyperlink text
     6       
     7        Removed tests for behaviors that we're no longer interested in:
     8        * editing/deleting/delete-link-1.html: Removed.
     9        * platform/mac/editing/deleting/delete-link-1-expected.checksum: Removed.
     10        * platform/mac/editing/deleting/delete-link-1-expected.png: Removed.
     11        * platform/mac/editing/deleting/delete-link-1-expected.txt: Removed.
     12        * editing/execCommand/19653-4-expected.txt: Removed.
     13        * editing/execCommand/19653-4.html: Removed.
     14       
     15        Reflects new behavior (don't remember removed anchors):
     16        * platform/mac/editing/deleting/5168598-expected.txt:
     17       
     18        Reflects new behavior.  Also made this test cross-platform:
     19        * editing/inserting/insert-before-link-1-expected.txt: Added.
     20        * editing/inserting/insert-before-link-1.html:
     21        * platform/mac/editing/inserting/insert-before-link-1-expected.checksum: Removed.
     22        * platform/mac/editing/inserting/insert-before-link-1-expected.png: Removed.
     23        * platform/mac/editing/inserting/insert-before-link-1-expected.txt: Removed.
     24
    1252008-11-21  Simon Fraser  <simon.fraser@apple.com>
    226

  • trunk/LayoutTests/editing/inserting/insert-before-link-1.html

    r17720 r38683  
    1 <p>This tests insertion before/after links in certain situations.  Inserting before a link should always put the text outside the link unless insertion is happening at the start of a paragraph.  Inserting after should always insert inside the link, unless insertion is happening at the end of the document.</p>
    2 <div id="div" contenteditable="true"><a id="link" href="http://www.google.com/">link.</a> This <a href="http://www.google.com/">This should.</a></div>
     1<div id="description">This tests insertion before/after links.  Text should always be inserted at the start or end of a link should be inserted outside of it.</div>
     2<div id="edit" contenteditable="true"><a id="link" href="http://www.google.com/">this should</a> <a href="http://www.google.com/">this should</a></div>
    33
    44<script>
    55
    66if (window.layoutTestController)
    7     layoutTestController.dumpEditingCallbacks();
     7    window.layoutTestController.dumpAsText();
    88   
    9 var div = document.getElementById("div");
     9var edit = document.getElementById("edit");
    1010var sel = window.getSelection();
    1111
    12 sel.setPosition(div, 0);
     12sel.setPosition(edit, 0);
    1313
    14 document.execCommand("InsertText", false, "This text should be in a ");
     14document.execCommand("InsertText", false, "this text should not be in a link");
    1515sel.modify("move", "forward", "word");
    1616sel.modify("move", "forward", "word");
    1717
    18 document.execCommand("InsertText", false, " should not.");
     18document.execCommand("InsertText", false, "this should not");
    1919
    20 sel.modify("move", "forward", "line");
     20sel.modify("move", "forward", "paragraphBoundary");
    2121
    22 document.execCommand("InsertText", false, " This should not.");
     22document.execCommand("InsertText", false, "this should not");
    2323
     24if (window.layoutTestController)
     25    document.body.innerText = document.getElementById("description").innerText + "\n\n" + edit.innerHTML;
    2426</script>

  • trunk/LayoutTests/platform/mac/editing/deleting/5168598-expected.txt

    r30635 r38683  
    1919layer at (13,83) size 142x13
    2020  RenderBlock {DIV} at (3,3) size 142x13
    21     RenderInline {FONT} at (0,0) size 0x13
    22       RenderInline {SPAN} at (0,0) size 0x13
    23         RenderBR {BR} at (1,0) size 0x13
    24 caret: position 0 of child 0 {BR} of child 0 {SPAN} of child 0 {FONT} of child 0 {DIV} of child 3 {INPUT} of child 0 {BODY} of child 0 {HTML} of document
     21    RenderBR {BR} at (1,0) size 0x13
     22caret: position 0 of child 0 {BR} of child 0 {DIV} of child 3 {INPUT} of child 0 {BODY} of child 0 {HTML} of document

  • trunk/WebCore/ChangeLog

    r38681 r38683  
     12008-11-21  Justin Garcia  <justin.garcia@apple.com>
     2
     3        Reviewed by Darin Adler.
     4
     5        <rdar://problem/5381788> Match NSTextView editing behavior at the end of hyperlink text
     6       
     7        Change link editing behavior to match TextEdit and MS Word when editing before and after
     8        a link (Pages has two caret positions at link boundaries, Thunderbird and FF behave like we
     9        used to, so it's difficult to get out of link editing mode):
     10        When inserting before or after a link, always insert content outside of the link.  This
     11        makes it impossible to get stuck in link editing mode, while making it slightly more
     12        difficult to edit link labels.  WebKit editors that care about this can add UI for editing
     13        link labels, like GMail and GoogleDocs have done.  We never actually had any bugs complaining
     14        about how it was difficult to edit link labels at the start/end, the code was just introduced
     15        with another bug fix without much thought.
     16       
     17        Don't remember removed links anymore, no other editor does this and it made it
     18        difficult/impossible to get out of link editing mode.  This code was added to fix
     19        <rdar://problem/4069359>, which is fixed instead by removing the styles from an
     20        enclosing anchor element from those styles that we remember when we delete content.
     21
     22        * editing/CompositeEditCommand.cpp:
     23        (WebCore::CompositeEditCommand::positionAvoidingSpecialElementBoundary):
     24        * editing/CompositeEditCommand.h:
     25        * editing/DeleteSelectionCommand.cpp:
     26        (WebCore::removeEnclosingAnchorStyle):
     27        (WebCore::DeleteSelectionCommand::saveTypingStyleState):
     28        (WebCore::DeleteSelectionCommand::doApply):
     29        * editing/DeleteSelectionCommand.h:
     30        * editing/EditCommand.cpp:
     31        (WebCore::EditCommand::apply):
     32        * editing/Editor.cpp:
     33        (WebCore::Editor::appliedEditing):
     34        * editing/InsertTextCommand.cpp:
     35        (WebCore::InsertTextCommand::prepareForTextInsertion):
     36        (WebCore::InsertTextCommand::input):
     37        * editing/RemoveFormatCommand.cpp:
     38        (WebCore::RemoveFormatCommand::doApply):
     39        * editing/SelectionController.cpp:
     40        (WebCore::SelectionController::setSelection):
     41        * editing/SelectionController.h:
     42
    1432008-11-21  Alice Liu  <alice.liu@apple.com>
    244

  • trunk/WebCore/editing/CompositeEditCommand.cpp

    r38648 r38683  
    958958// FIXME: This is only an approximation of NSTextViews insertion behavior, which varies depending on how
    959959// the caret was made.
    960 Position CompositeEditCommand::positionAvoidingSpecialElementBoundary(const Position& original, bool alwaysAvoidAnchors)
     960Position CompositeEditCommand::positionAvoidingSpecialElementBoundary(const Position& original)
    961961{
    962962    if (original.isNull())
    … …  
    972972        // If visually just after the anchor, insert *inside* the anchor unless it's the last
    973973        // VisiblePosition in the document, to match NSTextView.
    974         if (visiblePos == lastInAnchor && (isEndOfDocument(visiblePos) || alwaysAvoidAnchors)) {
     974        if (visiblePos == lastInAnchor) {
    975975            // Make sure anchors are pushed down before avoiding them so that we don't
    976976            // also avoid structural elements like lists and blocks (5142012).
    … …  
    991991        // If visually just before an anchor, insert *outside* the anchor unless it's the first
    992992        // VisiblePosition in a paragraph, to match NSTextView.
    993         if (visiblePos == firstInAnchor && (!isStartOfParagraph(visiblePos) || alwaysAvoidAnchors)) {
     993        if (visiblePos == firstInAnchor) {
    994994            // Make sure anchors are pushed down before avoiding them so that we don't
    995995            // also avoid structural elements like lists and blocks (5142012).

  • trunk/WebCore/editing/CompositeEditCommand.h

    r38648 r38683  
    104104    bool breakOutOfEmptyMailBlockquotedParagraph();
    105105   
    106     Position positionAvoidingSpecialElementBoundary(const Position&, bool alwaysAvoidAnchors = true);
     106    Position positionAvoidingSpecialElementBoundary(const Position&);
    107107   
    108108    PassRefPtr<Node> splitTreeToNode(Node*, Node*, bool splitAncestor = false);

  • trunk/WebCore/editing/DeleteSelectionCommand.cpp

    r38337 r38683  
    248248}
    249249
     250static void removeEnclosingAnchorStyle(CSSMutableStyleDeclaration* style, const Position& position)
     251{
     252    Node* enclosingAnchor = enclosingAnchorElement(position);
     253    if (!enclosingAnchor || !enclosingAnchor->parentNode())
     254        return;
     255           
     256    RefPtr<CSSMutableStyleDeclaration> parentStyle = Position(enclosingAnchor->parentNode(), 0).computedStyle()->copyInheritableProperties();
     257    RefPtr<CSSMutableStyleDeclaration> anchorStyle = Position(enclosingAnchor, 0).computedStyle()->copyInheritableProperties();
     258    parentStyle->diff(anchorStyle.get());
     259    anchorStyle->diff(style);
     260}
     261
    250262void DeleteSelectionCommand::saveTypingStyleState()
    251263{
    … …  
    263275    RefPtr<CSSComputedStyleDeclaration> computedStyle = positionBeforeTabSpan(m_selectionToDelete.start()).computedStyle();
    264276    m_typingStyle = computedStyle->copyInheritableProperties();
     277   
     278    removeEnclosingAnchorStyle(m_typingStyle.get(), m_selectionToDelete.start());
    265279   
    266280    // If we're deleting into a Mail blockquote, save the style at end() instead of start()
    … …  
    693707}
    694708
    695 void DeleteSelectionCommand::saveFullySelectedAnchor()
    696 {
    697     // If deleting an anchor element, save it away so that it can be restored
    698     // when the user begins entering text.
    699    
    700     Position start = m_selectionToDelete.start();
    701     Node* startAnchor = enclosingNodeWithTag(start.downstream(), aTag);
    702     if (!startAnchor)
    703         return;
    704        
    705     Position end = m_selectionToDelete.end();
    706     Node* endAnchor = enclosingNodeWithTag(end.upstream(), aTag);
    707     if (startAnchor != endAnchor)
    708         return;
    709 
    710     VisiblePosition visibleStart(m_selectionToDelete.visibleStart());
    711     VisiblePosition visibleEnd(m_selectionToDelete.visibleEnd());
    712 
    713     Node* beforeStartAnchor = enclosingNodeWithTag(visibleStart.previous().deepEquivalent().downstream(), aTag);
    714     Node* afterEndAnchor = enclosingNodeWithTag(visibleEnd.next().deepEquivalent().upstream(), aTag);
    715 
    716     if (startAnchor && startAnchor == endAnchor && startAnchor != beforeStartAnchor && endAnchor != afterEndAnchor)
    717         document()->frame()->editor()->setRemovedAnchor(startAnchor->cloneNode(false));
    718 }
    719 
    720709void DeleteSelectionCommand::doApply()
    721710{
    … …  
    763752    saveTypingStyleState();
    764753   
    765     saveFullySelectedAnchor();
    766    
    767754    // deleting just a BR is handled specially, at least because we do not
    768755    // want to replace it with a placeholder BR!

  • trunk/WebCore/editing/DeleteSelectionCommand.h

    r38337 r38683  
    5454    void initializePositionData();
    5555    void saveTypingStyleState();
    56     void saveFullySelectedAnchor();
    5756    void insertPlaceholderForAncestorBlockContent();
    5857    bool handleSpecialCaseBRDelete();

  • trunk/WebCore/editing/EditCommand.cpp

    r38094 r38683  
    8787    if (!m_parent)
    8888        updateLayout();
    89    
    90     // All high level commands, and all commands that a TypingCommand spawns, except for
    91     // text insertions, which should restore a removed anchor, should clear it.
    92     if (!m_parent && !isTypingCommand())
    93         frame->editor()->setRemovedAnchor(0);
    94     if (m_parent && m_parent->isTypingCommand() && !isInsertTextCommand())
    95         frame->editor()->setRemovedAnchor(0);
    9689
    9790    DeleteButtonController* deleteButtonController = frame->editor()->deleteButtonController();

  • trunk/WebCore/editing/Editor.cpp

    r38631 r38683  
    870870    // The old selection can be invalid here and calling shouldChangeSelection can produce some strange calls.
    871871    // See <rdar://problem/5729315> Some shouldChangeSelectedDOMRange contain Ranges for selections that are no longer valid
    872     // Don't clear the typing style or removedAnchor with this selection change.  We do those things elsewhere if necessary.
     872    // Don't clear the typing style with this selection change.  We do those things elsewhere if necessary.
    873873    if (newSelection == m_frame->selection()->selection() || m_frame->shouldChangeSelection(newSelection))
    874874        m_frame->selection()->setSelection(newSelection, false, false);

  • trunk/WebCore/editing/InsertTextCommand.cpp

    r38410 r38683  
    5757{
    5858    Position pos = p;
    59     // If an anchor was removed and the selection hasn't changed, we restore it.
    60     RefPtr<Node> anchor = document()->frame()->editor()->removedAnchor();
    61     if (anchor) {
    62         insertNodeAt(anchor.get(), pos);
    63         document()->frame()->editor()->setRemovedAnchor(0);
    64         pos = Position(anchor.get(), 0);
    65     }
    6659    // Prepare for text input by looking at the specified position.
    6760    // It may be necessary to insert a text node to receive characters.
    … …  
    149142        startPosition = startPosition.downstream();
    150143   
    151     // FIXME: This typing around anchor behavior doesn't exactly match TextEdit.  In TextEdit,
    152     // you won't be placed inside a link when typing after it if you've just placed the caret
    153     // there with the mouse.
    154     startPosition = positionAvoidingSpecialElementBoundary(startPosition, false);
     144    startPosition = positionAvoidingSpecialElementBoundary(startPosition);
    155145   
    156146    Position endPosition;

  • trunk/WebCore/editing/RemoveFormatCommand.cpp

    r34627 r38683  
    7272        return;
    7373   
    74     // Normally, deleting a fully selected anchor and then inserting text will re-create
    75     // the removed anchor, but we don't want that behavior here.
    76     frame->editor()->setRemovedAnchor(0);
    7774    // Insert the content with the default style.
    7875    // See <rdar://problem/5794382> RemoveFormat doesn't always reset text alignment

  • trunk/WebCore/editing/SelectionController.cpp

    r38098 r38683  
    9898}
    9999
    100 void SelectionController::setSelection(const Selection& s, bool closeTyping, bool clearTypingStyleAndRemovedAnchor, bool userTriggered)
     100void SelectionController::setSelection(const Selection& s, bool closeTyping, bool clearTypingStyle, bool userTriggered)
    101101{
    102102    if (m_isDragCaretController) {
    … …  
    113113   
    114114    if (s.base().node() && s.base().node()->document() != m_frame->document()) {
    115         s.base().node()->document()->frame()->selection()->setSelection(s, closeTyping, clearTypingStyleAndRemovedAnchor, userTriggered);
     115        s.base().node()->document()->frame()->selection()->setSelection(s, closeTyping, clearTypingStyle, userTriggered);
    116116        return;
    117117    }
    … …  
    120120        TypingCommand::closeTyping(m_frame->editor()->lastEditCommand());
    121121
    122     if (clearTypingStyleAndRemovedAnchor) {
     122    if (clearTypingStyle)
    123123        m_frame->clearTypingStyle();
    124         m_frame->editor()->setRemovedAnchor(0);
    125     }
    126124       
    127125    if (m_sel == s)

  • trunk/WebCore/editing/SelectionController.h

    r34900 r38683  
    5858
    5959    const Selection& selection() const { return m_sel; }
    60     void setSelection(const Selection&, bool closeTyping = true, bool clearTypingStyleAndRemovedAnchor = true, bool userTriggered = false);
     60    void setSelection(const Selection&, bool closeTyping = true, bool clearTypingStyle = true, bool userTriggered = false);
    6161    bool setSelectedRange(Range*, EAffinity, bool closeTyping);
    6262    void selectAll();
Note: See TracChangeset for help on using the changeset viewer.