Reviewed by John.
authorharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2005 20:30:12 +0000 (20:30 +0000)
committerharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 May 2005 20:30:12 +0000 (20:30 +0000)
        <rdar://problem/4120518> Mail: control-T in an empty message crashes mail

        * kwq/WebCoreBridge.mm:
        (-[WebCoreBridge rangeOfCharactersAroundCaret]):
        Nil-check result of VisiblePosition previous() and next().

        * khtml/editing/visible_position.cpp:
        (khtml::VisiblePosition::previous):
        (khtml::VisiblePosition::previousVisiblePosition):
        Make sure previous() does not return the original position.  Also, simplified.
        Commented odd, but required, behavior in previousVisiblePosition().

        * khtml/editing/visible_units.cpp:
        (khtml::startOfEditableContent):
        (khtml::endOfEditableContent):
        Removed redundant check for isEditableContent().

        * khtml/editing/jsediting.cpp:
        * khtml/khtml_part.cpp:
        (KHTMLPart::transpose):
        * khtml/khtml_part.h:
        * kwq/KWQKHTMLPart.h:
        * kwq/KWQKHTMLPart.mm:
        (KWQKHTMLPart::issueTransposeCommand):
        * layout-tests/editing/editing.js:
        * kwq/WebCoreBridge.h:
        Add support for transpose command in JavaScript and therefore layout tests.

        * layout-tests/editing/deleting/transpose-empty-expected.txt: Added.
        * layout-tests/editing/deleting/transpose-empty.html: Added.
        New test for this bug.

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

12 files changed:
LayoutTests/editing/deleting/transpose-empty-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/transpose-empty.html [new file with mode: 0644]
LayoutTests/editing/editing.js
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/jsediting.cpp
WebCore/khtml/editing/visible_position.cpp
WebCore/khtml/editing/visible_units.cpp
WebCore/khtml/khtml_part.cpp
WebCore/kwq/KWQKHTMLPart.h
WebCore/kwq/KWQKHTMLPart.mm
WebCore/kwq/WebCoreBridge.h
WebCore/kwq/WebCoreBridge.mm

diff --git a/LayoutTests/editing/deleting/transpose-empty-expected.txt b/LayoutTests/editing/deleting/transpose-empty-expected.txt
new file mode 100644 (file)
index 0000000..a11fff4
--- /dev/null
@@ -0,0 +1,6 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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
+caret: position 0 of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/deleting/transpose-empty.html b/LayoutTests/editing/deleting/transpose-empty.html
new file mode 100644 (file)
index 0000000..cacbd8b
--- /dev/null
@@ -0,0 +1,30 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    transposeCharactersCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="test"></body><script>runEditingTest();</script>
+</html>
index a1069ae2e2c56093dea50a1b1bad2c93730a0ad1..9032607956dec5a26b4394128f70ead3d8709a19 100644 (file)
@@ -9,6 +9,21 @@ var selection = window.getSelection();
 
 //-------------------------------------------------------------------------------------------------------
 
+function execTransposeCharactersCommand() {
+    document.execCommand("Transpose");
+}
+function transposeCharactersCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execTransposeCharactersCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execTransposeCharactersCommand();
+    }
+}
+
+//-------------------------------------------------------------------------------------------------------
+
 function execMoveSelectionForwardByCharacterCommand() {
     selection.modify("move", "forward", "character");
 }
index 4eaa8052e191083f96642bff524f3f148834511e..8ea35898c08cf4c6dc4c691d28914f587efb66b9 100644 (file)
@@ -1,3 +1,39 @@
+2005-05-26  David Harrison  <harrison@apple.com>
+
+        Reviewed by John.
+
+        <rdar://problem/4120518> Mail: control-T in an empty message crashes mail
+        
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge rangeOfCharactersAroundCaret]):
+        Nil-check result of VisiblePosition previous() and next().
+
+        * khtml/editing/visible_position.cpp:
+        (khtml::VisiblePosition::previous):
+        (khtml::VisiblePosition::previousVisiblePosition):
+        Make sure previous() does not return the original position.  Also, simplified.
+        Commented odd, but required, behavior in previousVisiblePosition().
+
+        * khtml/editing/visible_units.cpp:
+        (khtml::startOfEditableContent):
+        (khtml::endOfEditableContent):
+        Removed redundant check for isEditableContent().
+        
+        * khtml/editing/jsediting.cpp:
+        * khtml/khtml_part.cpp:
+        (KHTMLPart::transpose):
+        * khtml/khtml_part.h:
+        * kwq/KWQKHTMLPart.h:
+        * kwq/KWQKHTMLPart.mm:
+        (KWQKHTMLPart::issueTransposeCommand):
+        * layout-tests/editing/editing.js:
+        * kwq/WebCoreBridge.h:
+        Add support for transpose command in JavaScript and therefore layout tests.
+        
+        * layout-tests/editing/deleting/transpose-empty-expected.txt: Added.
+        * layout-tests/editing/deleting/transpose-empty.html: Added.
+        New test for this bug.
+        
 2005-05-24  Richard Williamson   <rjw@apple.com>
 
        Fixed <rdar://problem/4127061> <canvas> backing store should be zero filled
index 5431cb0f7921cf89528a5b7cdbdaceb91d9da537..1fcc7d3228e3bd7786624d329cb175153c779400 100644 (file)
@@ -338,6 +338,12 @@ bool execSuperscript(KHTMLPart *part, bool userInterface, const DOMString &value
     return execStyleChange(part, CSS_PROP_VERTICAL_ALIGN, "super");
 }
 
+bool execTranspose(KHTMLPart *part, bool userInterface, const DOMString &value)
+{
+    part->transpose();
+    return true;
+}
+
 bool execUnderline(KHTMLPart *part, bool userInterface, const DOMString &value)
 {
     bool isUnderlined = selectionStartHasStyle(part, CSS_PROP__KHTML_TEXT_DECORATIONS_IN_EFFECT, "underline");
@@ -519,6 +525,7 @@ QDict<CommandImp> createCommandDictionary()
         { "SelectAll", { execSelectAll, enabled, stateNone, valueNull } },
         { "Subscript", { execSubscript, enabledAnySelection, stateSubscript, valueNull } },
         { "Superscript", { execSuperscript, enabledAnySelection, stateSuperscript, valueNull } },
+        { "Transpose", { execTranspose, enabled, stateNone, valueNull } },
         { "Underline", { execUnderline, enabledAnySelection, stateUnderline, valueNull } },
         { "Undo", { execUndo, enabledUndo, stateNone, valueNull } },
         { "Unselect", { execUnselect, enabledAnySelection, stateNone, valueNull } }
index d1beb3ad624eea1bc8f3722b2d1f00c40250341d..43fb20a7fcbe988b857545512ac1a5568918c069 100644 (file)
@@ -158,8 +158,21 @@ VisiblePosition VisiblePosition::next() const
 
 VisiblePosition VisiblePosition::previous() const
 {
-    VisiblePosition result =  VisiblePosition(previousVisiblePosition(m_deepPosition), DOWNSTREAM);
-
+    // find first previous DOM position that is visible
+    Position pos = m_deepPosition;
+    while (!pos.atStart()) {
+        pos = pos.previous(UsingComposedCharacters);
+        if (isCandidate(pos))
+            break;
+    }
+    
+    // return null visible position if there is no previous visible position
+    if (pos.atStart())
+        return VisiblePosition();
+        
+    VisiblePosition result = VisiblePosition(pos, DOWNSTREAM);
+    ASSERT(result != *this);
+    
 #ifndef NDEBUG
     // we should always be able to make the affinity DOWNSTREAM, because going previous from an
     // UPSTREAM position can never yield another UPSTREAM position (unless line wrap length is 0!).
@@ -181,6 +194,12 @@ Position VisiblePosition::previousVisiblePosition(const Position &pos)
     Position downstreamTest = test.downstream();
     bool acceptAnyVisiblePosition = !isCandidate(test);
 
+    // NOTE: The first position examined is the deepEquivalent of pos.  This,
+    // of course, is often after pos in the DOM.  Some care is taken to not return
+    // a position later than pos, but it is clearly possible to return pos
+    // itself (if it is a candidate) when iterating back from a deepEquivalent
+    // that is not a candidate.  That is wrong!  However, our clients seem to
+    // like it.  Gotta lose those clients! (initDownstream and initUpstream)
     Position current = test;
     while (!current.atStart()) {
         current = current.previous(UsingComposedCharacters);
@@ -188,8 +207,8 @@ Position VisiblePosition::previousVisiblePosition(const Position &pos)
             return current;
         }
     }
-    
-    return Position();
+
+     return Position();
 }
 
 Position VisiblePosition::nextVisiblePosition(const Position &pos)
index c08fac741c4168f6172b2c016afdbfb611bea764..481a718836874d90b39446668458bb5c344083b8 100644 (file)
@@ -795,9 +795,6 @@ VisiblePosition startOfEditableContent(const VisiblePosition &c)
     if (!node)
         return VisiblePosition();
 
-    if (!node->isContentEditable())
-        return VisiblePosition();
-
     return VisiblePosition(node->rootEditableElement(), 0, DOWNSTREAM);
 }
 
@@ -808,9 +805,6 @@ VisiblePosition endOfEditableContent(const VisiblePosition &c)
     if (!node)
         return VisiblePosition();
 
-    if (!node->isContentEditable())
-        return VisiblePosition();
-
     node = node->rootEditableElement();
     if (!node)
         return VisiblePosition();
index edcb507e7297d8373e840312f69d81aa6d218a9a..8228156581f07cb88a9d88cc4bbcc3db8f9d92a1 100644 (file)
@@ -5451,6 +5451,13 @@ void KHTMLPart::pasteAndMatchStyle()
 #endif
 }
 
+void KHTMLPart::transpose()
+{
+#if APPLE_CHANGES
+    KWQ(this)->issueTransposeCommand();
+#endif
+}
+
 void KHTMLPart::redo()
 {
 #if APPLE_CHANGES
index 569f2e8ed2b24deeaff2aea1c4f3a73e9c79ef78..086daa493efb818648ba6afd5251589ea0285690 100644 (file)
@@ -339,6 +339,7 @@ public:
     void issueCopyCommand();
     void issuePasteCommand();
     void issuePasteAndMatchStyleCommand();
+    void issueTransposeCommand();
     void respondToChangedSelection(const khtml::Selection &oldSelection, bool closeTyping);
     void respondToChangedContents();
     virtual bool isContentEditable() const;
index 30e6f5520239e67381bab8e4dd866ad8b0f8f951..d782e6127c15073ca416f1a871e767fb916848d4 100644 (file)
@@ -3969,6 +3969,11 @@ void KWQKHTMLPart::issuePasteAndMatchStyleCommand()
     [_bridge issuePasteAndMatchStyleCommand];
 }
 
+void KWQKHTMLPart::issueTransposeCommand()
+{
+    [_bridge issueTransposeCommand];
+}
+
 bool KHTMLPart::canUndo() const
 {
     return [[KWQ(this)->_bridge undoManager] canUndo];
index 31056e5080703f8286959b611952214b2c355d8c..eee9946b8805758db8125a680f932160ad27be1e 100644 (file)
@@ -610,6 +610,7 @@ typedef enum
 - (void)issueCopyCommand;
 - (void)issuePasteCommand;
 - (void)issuePasteAndMatchStyleCommand;
+- (void)issueTransposeCommand;
 - (void)respondToChangedSelection;
 - (void)respondToChangedContents;
 - (void)setIsSelected:(BOOL)isSelected forView:(NSView *)view;
index 1759a4e90a506497114510055c6b8d6a413ee87b..314b7f495da2aff727dee4c1caf380cf66a3988b 100644 (file)
@@ -2256,7 +2256,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
     VisiblePosition caret(selection.start(), selection.startAffinity());
     VisiblePosition next = caret.next();
     VisiblePosition previous = caret.previous();
-    if (caret == next || caret == previous)
+    if (previous.isNull() || next.isNull() || caret == next || caret == previous)
         return nil;
 
     return [DOMRange _rangeWithImpl:makeRange(previous, next).get()];