Reviewed by Ken.
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Sep 2004 00:17:10 +0000 (00:17 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Sep 2004 00:17:10 +0000 (00:17 +0000)
        - improve spell checking so it doesn't unmark and remark as you move the cursor with the arrow

        * khtml/editing/htmlediting.h: Remove obsolete markMisspellingsInSelection private function.
        * khtml/editing/htmlediting.cpp:
        (khtml::EditCommand::EditCommand): Blow away the selection when starting an edit command.
        (khtml::EditCommand::apply): Remove code to mark misspellings because that's now done as part
        of blowing away the selection.
        (khtml::EditCommand::unapply): Blow away the selection instead of marking mispellings.
        (khtml::EditCommand::reapply): Ditto.
        (khtml::ReplaceSelectionCommand::doApply): Removed incorrect code that does spell checking
        on inserted text; this doesn't match NSText behavior.
        (khtml::TypingCommand::markMisspellingsAfterTyping): Use markMisspellingsInAdjacentWords
        function for greater clarity on what this actually does.

        * khtml/khtml_part.h: Replaced setSelection's "unmarkOldSelection" boolean parameter with a
        "keepTypingStyle" boolean parameter. Removed notifySelectionChanged.
        * khtml/khtml_part.cpp:
        (KHTMLPart::setSelection): Replaced the "unmarkOldSelection" boolean with a new "keepTypingStyle"
        boolean. This is a step on the way to simplifying how this works. Moved the code from the
        notifySelectionChanged function here, since there was no clear line between the two functions.
        (KHTMLPart::clearSelection): Call setSelection rather that having special case code here for
        the case of an empty selection.
        (KHTMLPart::appliedEditing): Remove the explicit "false" for "unmarkOldSelection".
        (KHTMLPart::unappliedEditing): Ditto.
        (KHTMLPart::reappliedEditing): Ditto.

        * kwq/KWQKHTMLPart.h: Replaced markMisspellingsInSelection and updateSpellChecking with
        markMisspellings and markMisspellingsInAdjacentWords. Added parameters to respondToChangedSelection.
        * kwq/KWQKHTMLPart.mm:
        (KWQKHTMLPart::markMisspellingsInAdjacentWords): Added.
        (KWQKHTMLPart::markMisspellings): New name for markMisspellingsInSelection. Simplified logic a
        bit and made it the caller's responsibility to expand the selection passed in to word boundaries.
        (KWQKHTMLPart::respondToChangedSelection): Added parameter that identifies the old selection,
        and changed logic so it won't mark mispellings in a word if the selection still starts in that word.

        - implemented empty-cells property in computed style

        * khtml/css/css_computedstyle.cpp: (DOM::CSSComputedStyleDeclarationImpl::getPropertyCSSValue):
        Added code for CSS_PROP_EMPTY_CELLS. I think the HTML converter in AppKit uses this.

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/css/css_computedstyle.cpp
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/htmlediting.h
WebCore/khtml/khtml_part.cpp
WebCore/khtml/khtml_part.h
WebCore/kwq/KWQKHTMLPart.h
WebCore/kwq/KWQKHTMLPart.mm

index aeb091008b0bde8769e52588c0810028cf33ceaa..1ece64107e43046a993b9491ce79992ed421dcd5 100644 (file)
@@ -1,3 +1,47 @@
+2004-09-28  Darin Adler  <darin@apple.com>
+
+        Reviewed by Ken.
+
+        - improve spell checking so it doesn't unmark and remark as you move the cursor with the arrow
+
+        * khtml/editing/htmlediting.h: Remove obsolete markMisspellingsInSelection private function.
+        * khtml/editing/htmlediting.cpp:
+        (khtml::EditCommand::EditCommand): Blow away the selection when starting an edit command.
+        (khtml::EditCommand::apply): Remove code to mark misspellings because that's now done as part
+        of blowing away the selection.
+        (khtml::EditCommand::unapply): Blow away the selection instead of marking mispellings.
+        (khtml::EditCommand::reapply): Ditto.
+        (khtml::ReplaceSelectionCommand::doApply): Removed incorrect code that does spell checking
+        on inserted text; this doesn't match NSText behavior.
+        (khtml::TypingCommand::markMisspellingsAfterTyping): Use markMisspellingsInAdjacentWords
+        function for greater clarity on what this actually does.
+
+        * khtml/khtml_part.h: Replaced setSelection's "unmarkOldSelection" boolean parameter with a
+        "keepTypingStyle" boolean parameter. Removed notifySelectionChanged.
+        * khtml/khtml_part.cpp:
+        (KHTMLPart::setSelection): Replaced the "unmarkOldSelection" boolean with a new "keepTypingStyle"
+        boolean. This is a step on the way to simplifying how this works. Moved the code from the
+        notifySelectionChanged function here, since there was no clear line between the two functions.
+        (KHTMLPart::clearSelection): Call setSelection rather that having special case code here for
+        the case of an empty selection.
+        (KHTMLPart::appliedEditing): Remove the explicit "false" for "unmarkOldSelection".
+        (KHTMLPart::unappliedEditing): Ditto.
+        (KHTMLPart::reappliedEditing): Ditto.
+
+        * kwq/KWQKHTMLPart.h: Replaced markMisspellingsInSelection and updateSpellChecking with
+        markMisspellings and markMisspellingsInAdjacentWords. Added parameters to respondToChangedSelection.
+        * kwq/KWQKHTMLPart.mm:
+        (KWQKHTMLPart::markMisspellingsInAdjacentWords): Added.
+        (KWQKHTMLPart::markMisspellings): New name for markMisspellingsInSelection. Simplified logic a
+        bit and made it the caller's responsibility to expand the selection passed in to word boundaries.
+        (KWQKHTMLPart::respondToChangedSelection): Added parameter that identifies the old selection,
+        and changed logic so it won't mark mispellings in a word if the selection still starts in that word.
+
+        - implemented empty-cells property in computed style
+
+        * khtml/css/css_computedstyle.cpp: (DOM::CSSComputedStyleDeclarationImpl::getPropertyCSSValue):
+        Added code for CSS_PROP_EMPTY_CELLS. I think the HTML converter in AppKit uses this.
+
 2004-09-28  Chris Blumenberg  <cblu@apple.com>
 
        Fixed n-squared issues with appending to KWQValueListImpl. This fixes the hang in 3794799.
index 4e695aadd50c49be3fd3e86c8bb803c1ffd02b07..0f83860c8f28355254c93c68e63d50d6b10aa8b6 100644 (file)
@@ -418,8 +418,14 @@ CSSValueImpl *CSSComputedStyleDeclarationImpl::getPropertyCSSValue(int propertyI
         ASSERT_NOT_REACHED();
         return 0;
     case CSS_PROP_EMPTY_CELLS:
-        // FIXME: unimplemented
-        break;
+        switch (style->emptyCells()) {
+            case khtml::SHOW:
+                return new CSSPrimitiveValueImpl(CSS_VAL_SHOW);
+            case khtml::HIDE:
+                return new CSSPrimitiveValueImpl(CSS_VAL_HIDE);
+        }
+        ASSERT_NOT_REACHED();
+        return 0;
     case CSS_PROP_FLOAT:
         switch (style->floating()) {
             case khtml::FNONE:
index 488caa37499229d4d4abba992e6cabe19adb1ce4..6ada4a77cb6e5280fc364e1c87d89601227e0941 100644 (file)
@@ -366,6 +366,8 @@ EditCommand::EditCommand(DocumentImpl *document)
     m_document->ref();
     m_startingSelection = m_document->part()->selection();
     m_endingSelection = m_startingSelection;
+
+    m_document->part()->setSelection(Selection(), false, true);
 }
 
 EditCommand::~EditCommand()
@@ -382,15 +384,9 @@ void EditCommand::apply()
     ASSERT(m_document->part());
     ASSERT(state() == NotApplied);
  
-    bool topLevel = !isCompositeStep();
     KHTMLPart *part = m_document->part();
 
-#if APPLE_CHANGES
-    if (topLevel) {
-        KWQ(part)->markMisspellingsInSelection(part->selection());
-    }
-#endif
+    ASSERT(part->selection().isNone());
 
     doApply();
     
@@ -401,7 +397,7 @@ void EditCommand::apply()
     if (!preservesTypingStyle())
         setTypingStyle(0);
 
-    if (topLevel) {
+    if (!isCompositeStep()) {
         document()->updateLayout();
         EditCommandPtr cmd(this);
         part->appliedEditing(cmd);
@@ -418,11 +414,10 @@ void EditCommand::unapply()
  
     KHTMLPart *part = m_document->part();
 
-#if APPLE_CHANGES
     if (topLevel) {
-        KWQ(part)->markMisspellingsInSelection(part->selection());
+        part->setSelection(Selection(), false, true);
     }
-#endif
+    ASSERT(part->selection().isNone());
     
     doUnapply();
     
@@ -445,12 +440,11 @@ void EditCommand::reapply()
  
     KHTMLPart *part = m_document->part();
 
-#if APPLE_CHANGES
     if (topLevel) {
-        KWQ(part)->markMisspellingsInSelection(part->selection());
+        part->setSelection(Selection(), false, true);
     }
-#endif
-
+    ASSERT(part->selection().isNone());
+    
     doReapply();
     
     m_state = Applied;
@@ -497,11 +491,6 @@ void EditCommand::setTypingStyle(CSSStyleDeclarationImpl *style)
         cmd->assignTypingStyle(style);
 }
 
-void EditCommand::markMisspellingsInSelection(const Selection &s)
-{
-    KWQ(document()->part())->markMisspellingsInSelection(s);
-}
-
 bool EditCommand::preservesTypingStyle() const
 {
     return false;
@@ -2022,10 +2011,6 @@ void ReplaceSelectionCommand::doApply()
             text += " ";
         }
         inputText(text, m_selectReplacement);
-        if (!m_selectReplacement) {
-            // Mark misspellings in the inserted content.
-            markMisspellingsInSelection(Selection(upstreamStart, endingSelection().extent()));
-        }
     } 
     else {
         // HTML fragment paste.
@@ -2074,7 +2059,6 @@ void ReplaceSelectionCommand::doApply()
             // Place the cursor after what was inserted, and mark misspellings in the inserted content.
             selection = Selection(Position(lastLeaf, lastLeaf->caretMaxOffset()));
             setEndingSelection(selection);
-            markMisspellingsInSelection(replacementSelection);
         }
     }
 }
@@ -2477,7 +2461,7 @@ void TypingCommand::markMisspellingsAfterTyping()
         VisiblePosition p1 = startOfWord(previous, LeftWordIfOnBoundary);
         VisiblePosition p2 = startOfWord(start, LeftWordIfOnBoundary);
         if (p1 != p2)
-            markMisspellingsInSelection(Selection(p1, start));
+            KWQ(document()->part())->markMisspellingsInAdjacentWords(p1);
     }
 }
 
index 3bef720d579c98aa68053ce1a005461aba91b7c1..fee32f9a5bedbc2a2692d17b371203a93b0ecc30 100644 (file)
@@ -142,8 +142,6 @@ public:
     DOM::CSSStyleDeclarationImpl *typingStyle() const { return m_typingStyle; };
     void setTypingStyle(DOM::CSSStyleDeclarationImpl *);
     
-    void markMisspellingsInSelection(const khtml::Selection &s);
-
     virtual bool isInputTextCommand() const;
     virtual bool isTypingCommand() const;
 
index c3955bb0c901d4c3e9e34f7574cb75d1985febe5..1f3aaf016f0bf50718ef6a65f7908907163a4931 100644 (file)
@@ -2341,22 +2341,39 @@ void KHTMLPart::setMark(const Selection &s)
     d->m_mark = s;
 }
 
-void KHTMLPart::setSelection(const Selection &s, bool closeTyping, bool unmarkOldSelection)
+void KHTMLPart::setSelection(const Selection &s, bool closeTyping, bool keepTypingStyle)
 {
-    if (d->m_selection != s) {
-        clearCaretRectIfNeeded(); 
+    if (d->m_selection == s) {
+        return;
+    }
+    
+    clearCaretRectIfNeeded();
+
 #if APPLE_CHANGES
-        // Mark misspellings in the soon-to-be previous selection.  When typing we check spelling
-        // elsewhere, so don't redo it here
-        if (unmarkOldSelection) {
-            KWQ(this)->markMisspellingsInSelection(d->m_selection);
-        }
+    Selection oldSelection = d->m_selection;
 #endif
-        d->m_selection = s;
-        if (!s.isNone())
-            setFocusNodeIfNeeded();
-        notifySelectionChanged(closeTyping);
-    }
+
+    d->m_selection = s;
+    if (!s.isNone())
+        setFocusNodeIfNeeded();
+
+    selectionLayoutChanged();
+
+    // Always clear the x position used for vertical arrow navigation.
+    // It will be restored by the vertical arrow navigation code if necessary.
+    d->m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation;
+
+    if (closeTyping)
+        TypingCommand::closeTyping(lastEditCommand());
+
+    if (!keepTypingStyle)
+        clearTypingStyle();
+    
+#if APPLE_CHANGES
+    KWQ(this)->respondToChangedSelection(oldSelection, closeTyping);
+#endif
+
+    emitSelectionChanged();
 }
 
 void KHTMLPart::setDragCaret(const Selection &dragCaret)
@@ -2370,11 +2387,7 @@ void KHTMLPart::setDragCaret(const Selection &dragCaret)
 
 void KHTMLPart::clearSelection()
 {
-    clearCaretRectIfNeeded();
-    bool hadSelection = hasSelection();
-    d->m_selection.clear();
-    if (hadSelection)
-        notifySelectionChanged();
+    setSelection(Selection());
 }
 
 void KHTMLPart::invalidateSelection()
@@ -2457,26 +2470,6 @@ void KHTMLPart::selectionLayoutChanged()
         d->m_doc->updateSelection();
 }
 
-void KHTMLPart::notifySelectionChanged(bool closeTyping)
-{
-    selectionLayoutChanged();
-
-    // Always clear the x position used for vertical arrow navigation.
-    // It will be restored by the vertical arrow navigation code if necessary.
-    d->m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation;
-
-    if (closeTyping)
-        TypingCommand::closeTyping(lastEditCommand());
-
-    clearTypingStyle();
-    
-    emitSelectionChanged();
-    
-#if APPLE_CHANGES
-    KWQ(this)->respondToChangedSelection();
-#endif
-}
-
 void KHTMLPart::setXPosForVerticalArrowNavigation(int x)
 {
     d->m_xPosForVerticalArrowNavigation = x;
@@ -4971,7 +4964,7 @@ EditCommandPtr KHTMLPart::lastEditCommand()
 
 void KHTMLPart::appliedEditing(EditCommandPtr &cmd)
 {
-    setSelection(cmd.endingSelection(), false, false);
+    setSelection(cmd.endingSelection(), false);
 
     // Now set the typing style from the command. Clear it when done.
     // This helps make the case work where you completely delete a piece
@@ -5003,7 +4996,7 @@ void KHTMLPart::appliedEditing(EditCommandPtr &cmd)
 
 void KHTMLPart::unappliedEditing(EditCommandPtr &cmd)
 {
-    setSelection(cmd.startingSelection(), true, false);
+    setSelection(cmd.startingSelection(), true);
 #if APPLE_CHANGES
     KWQ(this)->registerCommandForRedo(cmd);
     KWQ(this)->respondToChangedContents();
@@ -5013,7 +5006,7 @@ void KHTMLPart::unappliedEditing(EditCommandPtr &cmd)
 
 void KHTMLPart::reappliedEditing(EditCommandPtr &cmd)
 {
-    setSelection(cmd.endingSelection(), true, false);
+    setSelection(cmd.endingSelection(), true);
 #if APPLE_CHANGES
     KWQ(this)->registerCommandForUndo(cmd);
     KWQ(this)->respondToChangedContents();
index 3be138bba6d6012c9a8467e582c7e2f85096e05d..ea5b994a3b081fc395f9e5ad4cc6c0f8fde3ba0b 100644 (file)
@@ -589,7 +589,7 @@ public:
   /**
    * Sets the current selection.
    */
-  void setSelection(const khtml::Selection &, bool closeTyping = true, bool unmarkOldSelection = true);
+  void setSelection(const khtml::Selection &, bool closeTyping = true, bool keepTypingStyle = false);
 
   /**
    * Returns a mark, to be used as emacs uses it.
@@ -1192,11 +1192,6 @@ private:
    */
   void selectionLayoutChanged();
 
-  /**
-   * @internal
-   */
-  void notifySelectionChanged(bool closeTyping=true);
-
   /**
    * @internal
    */
index 109ffced039b52b4897f3c862b4f99b4e7e3fd72..45b18d5d498ce852284355efaf33078ed310be57 100644 (file)
@@ -54,6 +54,7 @@ namespace DOM {
 
 namespace khtml {
     class RenderObject;
+    class VisiblePosition;
     struct DashboardRegionValue;
 }
 
@@ -229,8 +230,8 @@ public:
     NSImage *snapshotDragImage(DOM::Node node, NSRect *imageRect, NSRect *elementRect) const;
 
     NSFont *fontForSelection(bool *hasMultipleFonts) const;
-    void markMisspellingsInSelection(const khtml::Selection &selection);
-    void updateSpellChecking();
+    void markMisspellingsInAdjacentWords(const khtml::VisiblePosition &);
+    void markMisspellings(const khtml::Selection &);
 
     NSFileWrapper *fileWrapperForElement(DOM::ElementImpl *);
     NSAttributedString *attributedString(DOM::NodeImpl *startNode, int startOffset, DOM::NodeImpl *endNode, int endOffset);
@@ -322,7 +323,7 @@ public:
     void issueCutCommand();
     void issueCopyCommand();
     void issuePasteCommand();
-    void respondToChangedSelection();
+    void respondToChangedSelection(const khtml::Selection &oldSelection, bool closeTyping);
     void respondToChangedContents();
     bool isContentEditable() const;
     bool shouldBeginEditing(const DOM::Range &) const;
index c0d256f1e11aea29c49e400ec65a25e05a1e0cac..e0858c83d0a0b07d12c055bc78fa5de3f4c51912 100644 (file)
@@ -3787,38 +3787,30 @@ bool KHTMLPart::canRedo() const
     return [[KWQ(this)->_bridge undoManager] canRedo];
 }
 
-void KWQKHTMLPart::markMisspellingsInSelection(const Selection &selection)
+void KWQKHTMLPart::markMisspellingsInAdjacentWords(const VisiblePosition &p)
 {
-    // No work to do if there is no selection or continuous spell check is off.
-    if (selection.isNone() || ![_bridge isContinuousSpellCheckingEnabled])
+    if (![_bridge isContinuousSpellCheckingEnabled])
         return;
+    markMisspellings(Selection(startOfWord(p, LeftWordIfOnBoundary), endOfWord(p, RightWordIfOnBoundary)));
+}
+
+void KWQKHTMLPart::markMisspellings(const Selection &selection)
+{
+    // This function is called with a selection already expanded to word boundaries.
+    // Might be nice to assert that here.
 
-    // Expand selection to word boundaries so that complete words wind up being passed to the spell checker.
-    //
-    // FIXME: It seems that NSSpellChecker is too slow to handle finding multiple misspellings in an
-    // arbitrarily large selection.
-    // So, for now, the idea is to mimic AppKit behavior and limit the selection to the first word 
-    // of the selection passed in.
-    // This is not ideal by any means, but this is the convention.
-    VisiblePosition start(selection.start());
-    Selection s(startOfWord(start, LeftWordIfOnBoundary), endOfWord(start, RightWordIfOnBoundary));
-    if (!s.isRange())
+    if (![_bridge isContinuousSpellCheckingEnabled])
         return;
-    // Change to this someday to spell check the entire selection.
-    // The rest of this function is prepared to handle finding multiple misspellings in a 
-    // more-than-one-word selection.
 
-    Range searchRange(s.toRange());
+    Range searchRange(selection.toRange());
+    if (searchRange.isNull() || searchRange.isDetached())
+        return;
     
     // If we're not in an editable node, bail.
     NodeImpl *editableNodeImpl = searchRange.startContainer().handle();
     if (!editableNodeImpl->isContentEditable())
         return;
     
-    if (searchRange.collapsed())
-        // nothing to search in
-        return;       
-    
     NSSpellChecker *checker = [NSSpellChecker sharedSpellChecker];
     WordAwareIterator it(searchRange);
     
@@ -3855,26 +3847,39 @@ void KWQKHTMLPart::markMisspellingsInSelection(const Selection &selection)
     }
 }
 
-void KWQKHTMLPart::updateSpellChecking()
+void KWQKHTMLPart::respondToChangedSelection(const Selection &oldSelection, bool closeTyping)
 {
     if (xmlDocImpl()) {
         if ([_bridge isContinuousSpellCheckingEnabled]) {
-            // This only erases a marker in the first word of the selection.  Perhaps peculiar, but it
-            // matches AppKit.
-            VisiblePosition start(selection().start());
-            Selection s(startOfWord(start, LeftWordIfOnBoundary), endOfWord(start, RightWordIfOnBoundary));
-            xmlDocImpl()->removeMarker(s.toRange(), DocumentMarker::Spelling);
-        }
-        else {
+            VisiblePosition oldStart(oldSelection.start());
+            Selection oldAdjacentWords(startOfWord(oldStart, LeftWordIfOnBoundary), endOfWord(oldStart, RightWordIfOnBoundary));
+
+            VisiblePosition newStart(selection().start());
+            Selection newAdjacentWords(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
+
+            if (oldAdjacentWords != newAdjacentWords) {
+                // Mark misspellings in the portion that was previously unmarked because of
+                // the proximity of the start of the selection. We only spell check words in
+                // the vicinity of the start of the old selection because the spelling checker
+                // is not fast enough to do a lot of spelling checking implicitly. This matches
+                // AppKit. This function is really the only code that knows that rule. The
+                // markMisspellings function is prepared to handler larger ranges.
+
+                // When typing we check spelling elsewhere, so don't redo it here.
+                if (closeTyping) {
+                    markMisspellings(oldAdjacentWords);
+                }
+
+                // This only erases a marker in the first word of the selection.
+                // Perhaps peculiar, but it matches AppKit.
+                xmlDocImpl()->removeMarker(newAdjacentWords.toRange(), DocumentMarker::Spelling);
+            }
+        } else {
             // When continuous spell checking is off, no markers appear after the selection changes.
             xmlDocImpl()->removeAllMarkers();
         }
     }
-}
 
-void KWQKHTMLPart::respondToChangedSelection()
-{
-    updateSpellChecking();
     [_bridge respondToChangedSelection];
 }