[webkit-changes] cvs commit: WebCore/kwq KWQKHTMLPart.mm
WebCoreBridge.mm
David
harrison at opensource.apple.com
Tue Sep 13 20:43:03 PDT 2005
harrison 05/09/13 20:43:03
Modified: . ChangeLog
khtml/editing split_text_node_command.cpp
khtml/xml dom_docimpl.cpp dom_docimpl.h dom_textimpl.cpp
kwq KWQKHTMLPart.mm WebCoreBridge.mm
Added: manual-tests keep_spelling_markers.html
Log:
Reviewed by Justin.
<rdar://problem/4204892> Correcting incorrect spelling in Mail causes others to lose their red squiggles
Fixed by having SplitTextNodeCommand::doApply copy the markers to the new node, and having SplitTextNodeCommand::doUnapply
copy the markers from the merged-in node to the combined node.
btw - filed <rdar://problem/4256492> "WebCore editing automated layout tests verify spelling markers"
so that we can automate tests for this.
Test cases added:
* manual-tests/keep_spelling_markers.html: Added.
* khtml/editing/split_text_node_command.cpp:
(khtml::SplitTextNodeCommand::doApply):
(khtml::SplitTextNodeCommand::doUnapply):
Call new copyMarkers() function.
* khtml/xml/dom_docimpl.cpp:
(DocumentImpl::removeMarkers):
(DocumentImpl::addMarker):
(DocumentImpl::copyMarkers):
(DocumentImpl::markersForNode):
(DocumentImpl::shiftMarkers):
* khtml/xml/dom_docimpl.h:
(DOM::DocumentMarker::):
Added copyMarkers() function. Reorganized code for clarity.
* khtml/xml/dom_textimpl.cpp:
(CharacterDataImpl::setData):
(CharacterDataImpl::deleteData):
(CharacterDataImpl::replaceData):
* kwq/KWQKHTMLPart.mm:
(KWQKHTMLPart::respondToChangedSelection):
* kwq/WebCoreBridge.mm:
(-[WebCoreBridge unmarkAllMisspellings]):
Update calls to marker functions.
Revision Changes Path
1.118 +40 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.117
retrieving revision 1.118
diff -u -r1.117 -r1.118
--- ChangeLog 12 Sep 2005 21:53:30 -0000 1.117
+++ ChangeLog 14 Sep 2005 03:43:00 -0000 1.118
@@ -1,3 +1,43 @@
+2005-09-13 David Harrison <harrison at apple.com>
+
+ Reviewed by Justin.
+
+ <rdar://problem/4204892> Correcting incorrect spelling in Mail causes others to lose their red squiggles
+
+ Fixed by having SplitTextNodeCommand::doApply copy the markers to the new node, and having SplitTextNodeCommand::doUnapply
+ copy the markers from the merged-in node to the combined node.
+
+ btw - filed <rdar://problem/4256492> "WebCore editing automated layout tests verify spelling markers"
+ so that we can automate tests for this.
+
+ Test cases added:
+ * manual-tests/keep_spelling_markers.html: Added.
+
+ * khtml/editing/split_text_node_command.cpp:
+ (khtml::SplitTextNodeCommand::doApply):
+ (khtml::SplitTextNodeCommand::doUnapply):
+ Call new copyMarkers() function.
+
+ * khtml/xml/dom_docimpl.cpp:
+ (DocumentImpl::removeMarkers):
+ (DocumentImpl::addMarker):
+ (DocumentImpl::copyMarkers):
+ (DocumentImpl::markersForNode):
+ (DocumentImpl::shiftMarkers):
+ * khtml/xml/dom_docimpl.h:
+ (DOM::DocumentMarker::):
+ Added copyMarkers() function. Reorganized code for clarity.
+
+ * khtml/xml/dom_textimpl.cpp:
+ (CharacterDataImpl::setData):
+ (CharacterDataImpl::deleteData):
+ (CharacterDataImpl::replaceData):
+ * kwq/KWQKHTMLPart.mm:
+ (KWQKHTMLPart::respondToChangedSelection):
+ * kwq/WebCoreBridge.mm:
+ (-[WebCoreBridge unmarkAllMisspellings]):
+ Update calls to marker functions.
+
2005-09-12 Eric Seidel <eseidel at apple.com>
No review, build fix, only affects SVG.
1.1 WebCore/manual-tests/keep_spelling_markers.html
Index: keep_spelling_markers.html
===================================================================
<HTML><BODY contenteditable>
<p>- open this in Blot</p>
<p>- turn on Check Spelling as You Type from Edit->Spelling menu</p>
<p>- click on each of the misspelled words in the line at the bottom of this page</p>
<p> (result should be that each misspelled word now has a red squiggly line below it)
<p>- control-click on “foor” in that sentence</p>
<p>- select a corrected spelling from the popup, e.g.“four”</p>
<p>- make sure that no other misspellings lost their red squiggly underline</p>
<p>- was bug #4204892</p>
<hr>
<p>One twwo three foor five siz seven</p>
</BODY></HTML>
1.2 +4 -2 WebCore/khtml/editing/split_text_node_command.cpp
Index: split_text_node_command.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/editing/split_text_node_command.cpp,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- split_text_node_command.cpp 24 May 2005 07:21:47 -0000 1.1
+++ split_text_node_command.cpp 14 Sep 2005 03:43:01 -0000 1.2
@@ -78,6 +78,7 @@
m_text1->ref();
}
+ document()->copyMarkers(m_text2, 0, m_offset, m_text1, 0);
m_text2->deleteData(0, m_offset, exceptionCode);
ASSERT(exceptionCode == 0);
@@ -92,13 +93,14 @@
{
ASSERT(m_text1);
ASSERT(m_text2);
-
ASSERT(m_text1->nextSibling() == m_text2);
-
+
int exceptionCode = 0;
m_text2->insertData(0, m_text1->data(), exceptionCode);
ASSERT(exceptionCode == 0);
+ document()->copyMarkers(m_text1, 0, m_offset, m_text2, 0);
+
m_text2->parentNode()->removeChild(m_text1, exceptionCode);
ASSERT(exceptionCode == 0);
1.254 +109 -53 WebCore/khtml/xml/dom_docimpl.cpp
Index: dom_docimpl.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_docimpl.cpp,v
retrieving revision 1.253
retrieving revision 1.254
diff -u -r1.253 -r1.254
--- dom_docimpl.cpp 10 Sep 2005 20:47:14 -0000 1.253
+++ dom_docimpl.cpp 14 Sep 2005 03:43:01 -0000 1.254
@@ -2776,28 +2776,26 @@
}
}
-void DocumentImpl::removeMarker(RangeImpl *range, DocumentMarker::MarkerType type)
+void DocumentImpl::removeMarkers(RangeImpl *range, DocumentMarker::MarkerType markerType)
{
// Use a TextIterator to visit the potentially multiple nodes the range covers.
for (TextIterator markedText(range); !markedText.atEnd(); markedText.advance()) {
SharedPtr<RangeImpl> textPiece = markedText.range();
int exception = 0;
- DocumentMarker marker = {type, textPiece->startOffset(exception), textPiece->endOffset(exception)};
- removeMarker(textPiece->startContainer(exception), marker);
+ long startOffset = textPiece->startOffset(exception);
+ long length = textPiece->endOffset(exception) - startOffset + 1;
+ removeMarkers(textPiece->startContainer(exception), startOffset, length, markerType);
}
}
-// FIXME: We don't deal with markers of more than one type yet
-
// Markers are stored in order sorted by their location. They do not overlap each other, as currently
// required by the drawing code in render_text.cpp.
void DocumentImpl::addMarker(NodeImpl *node, DocumentMarker newMarker)
{
assert(newMarker.endOffset >= newMarker.startOffset);
- if (newMarker.endOffset == newMarker.startOffset) {
- return; // zero length markers are a NOP
- }
+ if (newMarker.endOffset == newMarker.startOffset)
+ return;
QValueList <DocumentMarker> *markers = m_markers.find(node);
if (!markers) {
@@ -2837,48 +2835,92 @@
node->renderer()->repaint();
}
-void DocumentImpl::removeMarker(NodeImpl *node, DocumentMarker target)
+// copies markers from srcNode to dstNode, applying the specified shift delta to the copies. The shift is
+// useful if, e.g., the caller has created the dstNode from a non-prefix substring of the srcNode.
+void DocumentImpl::copyMarkers(NodeImpl *srcNode, ulong startOffset, long length, NodeImpl *dstNode, long delta, DocumentMarker::MarkerType markerType)
{
- assert(target.endOffset >= target.startOffset);
- if (target.endOffset == target.startOffset) {
- return; // zero length markers are a NOP
+ if (length <= 0)
+ return;
+
+ QValueList <DocumentMarker> *markers = m_markers.find(srcNode);
+ if (!markers)
+ return;
+
+ bool docDirty = false;
+ ulong endOffset = startOffset + length - 1;
+ QValueListIterator<DocumentMarker> it;
+ for (it = markers->begin(); it != markers->end(); ++it) {
+ DocumentMarker marker = *it;
+
+ // stop if we are now past the specified range
+ if (marker.startOffset > endOffset)
+ break;
+
+ // skip marker that is before the specified range or is the wrong type
+ if (marker.endOffset < startOffset || (marker.type != markerType && markerType != DocumentMarker::AllMarkers))
+ continue;
+
+ // pin the marker to the specified range and apply the shift delta
+ docDirty = true;
+ if (marker.startOffset < startOffset)
+ marker.startOffset = startOffset;
+ if (marker.endOffset > endOffset)
+ marker.endOffset = endOffset;
+ marker.startOffset += delta;
+ marker.endOffset += delta;
+
+ addMarker(dstNode, marker);
}
+
+ // repaint the affected node
+ if (docDirty && dstNode->renderer())
+ dstNode->renderer()->repaint();
+}
+
+void DocumentImpl::removeMarkers(NodeImpl *node, ulong startOffset, long length, DocumentMarker::MarkerType markerType)
+{
+ if (length <= 0)
+ return;
QValueList <DocumentMarker> *markers = m_markers.find(node);
- if (!markers) {
+ if (!markers)
return;
- }
bool docDirty = false;
+ ulong endOffset = startOffset + length - 1;
QValueListIterator<DocumentMarker> it;
for (it = markers->begin(); it != markers->end(); ) {
DocumentMarker marker = *it;
- if (target.endOffset <= marker.startOffset) {
- // This is the first marker that is completely after target. All done.
+ // markers are returned in order, so stop if we are now past the specified range
+ if (marker.startOffset > endOffset)
break;
- } else if (target.startOffset >= marker.endOffset) {
- // marker is before target. Keep scanning.
+
+ // skip marker that is wrong type or before target
+ if (marker.endOffset < startOffset || (marker.type != markerType && markerType != DocumentMarker::AllMarkers)) {
it++;
- } else {
- // at this point we know that marker and target intersect in some way
- docDirty = true;
+ continue;
+ }
- // pitch the old marker
- it = markers->remove(it);
- // it now points to the next node
-
- // add either of the resulting slices that are left after removing target
- if (target.startOffset > marker.startOffset) {
- DocumentMarker newLeft = marker;
- newLeft.endOffset = target.startOffset;
- markers->insert(it, newLeft);
- }
- if (marker.endOffset > target.endOffset) {
- DocumentMarker newRight = marker;
- newRight.startOffset = target.endOffset;
- markers->insert(it, newRight);
- }
+ // at this point we know that marker and target intersect in some way
+ docDirty = true;
+
+ // pitch the old marker
+ it = markers->remove(it);
+ // it now points to the next node
+
+ // add either of the resulting slices that are left after removing target
+ // NOTE: This adds to the list we are iterating! That is OK regardless of
+ // whether the iterator sees the new node, since the new node is a keeper.
+ if (startOffset > marker.startOffset) {
+ DocumentMarker newLeft = marker;
+ newLeft.endOffset = startOffset;
+ markers->insert(it, newLeft);
+ }
+ if (marker.endOffset > endOffset) {
+ DocumentMarker newRight = marker;
+ newRight.startOffset = endOffset;
+ markers->insert(it, newRight);
}
}
@@ -2893,21 +2935,13 @@
QValueList<DocumentMarker> DocumentImpl::markersForNode(NodeImpl *node)
{
QValueList <DocumentMarker> *markers = m_markers.find(node);
- if (markers) {
+ if (markers)
return *markers;
- } else {
- return QValueList <DocumentMarker> ();
- }
-}
-void DocumentImpl::removeAllMarkers(NodeImpl *node, ulong startOffset, long length)
-{
- // FIXME - yet another cheat that relies on us only having one marker type
- DocumentMarker marker = {DocumentMarker::Spelling, startOffset, startOffset+length};
- removeMarker(node, marker);
+ return QValueList <DocumentMarker> ();
}
-void DocumentImpl::removeAllMarkers(NodeImpl *node)
+void DocumentImpl::removeMarkers(NodeImpl *node)
{
QValueList<DocumentMarker> *markers = m_markers.take(node);
if (markers) {
@@ -2918,18 +2952,40 @@
}
}
-void DocumentImpl::removeAllMarkers()
+void DocumentImpl::removeMarkers(DocumentMarker::MarkerType markerType)
{
- QPtrDictIterator< QValueList<DocumentMarker> > it(m_markers);
- for (; NodeImpl *node = static_cast<NodeImpl *>(it.currentKey()); ++it) {
+ // outer loop: process each markered node in the document
+ QPtrDictIterator< QValueList<DocumentMarker> > dictIterator(m_markers);
+ for (; NodeImpl *node = static_cast<NodeImpl *>(dictIterator.currentKey()); ) {
+ // inner loop: process each marker in the current node
+ QValueList <DocumentMarker> *markers = static_cast<QValueList <DocumentMarker> *>(dictIterator.current());
+ QValueListIterator<DocumentMarker> markerIterator;
+ for (markerIterator = markers->begin(); markerIterator != markers->end(); ) {
+ DocumentMarker marker = *markerIterator;
+
+ // skip nodes that are not of the specified type
+ if (marker.type != markerType && markerType != DocumentMarker::AllMarkers) {
+ ++markerIterator;
+ continue;
+ }
+
+ // pitch the old marker
+ markerIterator = markers->remove(markerIterator);
+ // markerIterator now points to the next node
+ }
+
+ // delete the node's list if it is now empty
+ if (markers->isEmpty())
+ m_markers.remove(node);
+
+ // cause the node to be redrawn
RenderObject *renderer = node->renderer();
if (renderer)
renderer->repaint();
}
- m_markers.clear();
}
-void DocumentImpl::shiftMarkers(NodeImpl *node, ulong startOffset, long delta)
+void DocumentImpl::shiftMarkers(NodeImpl *node, ulong startOffset, long delta, DocumentMarker::MarkerType markerType)
{
QValueList <DocumentMarker> *markers = m_markers.find(node);
if (!markers)
@@ -2939,7 +2995,7 @@
QValueListIterator<DocumentMarker> it;
for (it = markers->begin(); it != markers->end(); ++it) {
DocumentMarker &marker = *it;
- if (marker.startOffset >= startOffset) {
+ if (marker.startOffset >= startOffset && (markerType == DocumentMarker::AllMarkers || marker.type == markerType)) {
assert((int)marker.startOffset + delta >= 0);
marker.startOffset += delta;
marker.endOffset += delta;
1.125 +9 -7 WebCore/khtml/xml/dom_docimpl.h
Index: dom_docimpl.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_docimpl.h,v
retrieving revision 1.124
retrieving revision 1.125
diff -u -r1.124 -r1.125
--- dom_docimpl.h 31 Aug 2005 04:38:41 -0000 1.124
+++ dom_docimpl.h 14 Sep 2005 03:43:01 -0000 1.125
@@ -108,7 +108,8 @@
struct DocumentMarker
{
enum MarkerType {
- Spelling
+ AllMarkers = -1,
+ Spelling = 0
// Not doing grammar yet, but this is a placeholder for it
// Grammar
};
@@ -534,13 +535,14 @@
DOMString queryCommandValue(const DOMString &command);
void addMarker(RangeImpl *range, DocumentMarker::MarkerType type);
- void removeMarker(RangeImpl *range, DocumentMarker::MarkerType type);
void addMarker(NodeImpl *node, DocumentMarker marker);
- void removeMarker(NodeImpl *node, DocumentMarker marker);
- void removeAllMarkers(NodeImpl *node, ulong startOffset, long length);
- void removeAllMarkers(NodeImpl *node);
- void removeAllMarkers();
- void shiftMarkers(NodeImpl *node, ulong startOffset, long delta);
+ void copyMarkers(NodeImpl *srcNode, ulong startOffset, long length, NodeImpl *dstNode, long delta, DocumentMarker::MarkerType markerType=DocumentMarker::AllMarkers);
+ void removeMarkers(RangeImpl *range, DocumentMarker::MarkerType markerType=DocumentMarker::AllMarkers);
+ void removeMarkers(NodeImpl *node, ulong startOffset, long length, DocumentMarker::MarkerType markerType=DocumentMarker::AllMarkers);
+ void removeMarkers(DocumentMarker::MarkerType markerType=DocumentMarker::AllMarkers);
+ void removeMarkers(NodeImpl *node);
+ void shiftMarkers(NodeImpl *node, ulong startOffset, long delta, DocumentMarker::MarkerType markerType=DocumentMarker::AllMarkers);
+
QValueList<DocumentMarker> markersForNode(NodeImpl *node);
/**
1.31 +3 -3 WebCore/khtml/xml/dom_textimpl.cpp
Index: dom_textimpl.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_textimpl.cpp,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -r1.30 -r1.31
--- dom_textimpl.cpp 31 Aug 2005 04:38:42 -0000 1.30
+++ dom_textimpl.cpp 14 Sep 2005 03:43:01 -0000 1.31
@@ -77,7 +77,7 @@
dispatchModifiedEvent(oldStr);
if(oldStr) oldStr->deref();
- getDocument()->removeAllMarkers(this);
+ getDocument()->removeMarkers(this);
}
unsigned long CharacterDataImpl::length() const
@@ -156,7 +156,7 @@
oldStr->deref();
// update the markers for spell checking and grammar checking
- getDocument()->removeAllMarkers(this, offset, count);
+ getDocument()->removeMarkers(this, offset, count);
getDocument()->shiftMarkers(this, offset + count, -count);
}
@@ -186,7 +186,7 @@
// update the markers for spell checking and grammar checking
int diff = arg.length() - count;
- getDocument()->removeAllMarkers(this, offset, count);
+ getDocument()->removeMarkers(this, offset, count);
getDocument()->shiftMarkers(this, offset + count, diff);
}
1.671 +2 -2 WebCore/kwq/KWQKHTMLPart.mm
Index: KWQKHTMLPart.mm
===================================================================
RCS file: /cvs/root/WebCore/kwq/KWQKHTMLPart.mm,v
retrieving revision 1.670
retrieving revision 1.671
diff -u -r1.670 -r1.671
--- KWQKHTMLPart.mm 9 Sep 2005 18:34:51 -0000 1.670
+++ KWQKHTMLPart.mm 14 Sep 2005 03:43:02 -0000 1.671
@@ -4092,11 +4092,11 @@
// This only erases a marker in the first word of the selection.
// Perhaps peculiar, but it matches AppKit.
- xmlDocImpl()->removeMarker(newAdjacentWords.toRange().get(), DocumentMarker::Spelling);
+ xmlDocImpl()->removeMarkers(newAdjacentWords.toRange().get(), DocumentMarker::Spelling);
}
} else {
// When continuous spell checking is off, no markers appear after the selection changes.
- xmlDocImpl()->removeAllMarkers();
+ xmlDocImpl()->removeMarkers(DocumentMarker::Spelling);
}
}
1.412 +1 -1 WebCore/kwq/WebCoreBridge.mm
Index: WebCoreBridge.mm
===================================================================
RCS file: /cvs/root/WebCore/kwq/WebCoreBridge.mm,v
retrieving revision 1.411
retrieving revision 1.412
diff -u -r1.411 -r1.412
--- WebCoreBridge.mm 1 Sep 2005 18:03:35 -0000 1.411
+++ WebCoreBridge.mm 14 Sep 2005 03:43:02 -0000 1.412
@@ -1141,7 +1141,7 @@
if (!doc) {
return;
}
- doc->removeAllMarkers();
+ doc->removeMarkers(DOM::DocumentMarker::Spelling);
}
- (void)setTextSizeMultiplier:(float)multiplier
More information about the webkit-changes
mailing list