[webkit-changes] cvs commit: WebCore/khtml/xml dom_docimpl.cpp
dom_docimpl.h dom_nodeimpl.cpp
David
harrison at opensource.apple.com
Fri Nov 18 13:34:43 PST 2005
harrison 05/11/18 13:34:43
Modified: . ChangeLog
khtml/xml dom_docimpl.cpp dom_docimpl.h dom_nodeimpl.cpp
Log:
<http://bugzilla.opendarwin.org/show_bug.cgi?id=5629>
REGRESSION: appendChild() does not remove nodes from source nodelist when inserting into destination
Enable event dispatch when calling removeChild() in loops. That it was
disabled previously was wrong because the DOM is not fragile at that point.
Makes the event dispatch forbiddance a debug-only check
* khtml/xml/dom_docimpl.cpp:
(DocumentImpl::createEvent):
* khtml/xml/dom_docimpl.h:
* khtml/xml/dom_nodeimpl.cpp:
(DOM::NodeImpl::dispatchEvent):
(DOM::NodeImpl::dispatchGenericEvent):
(DOM::NodeImpl::dispatchHTMLEvent):
(DOM::NodeImpl::dispatchWindowEvent):
(DOM::NodeImpl::dispatchMouseEvent):
(DOM::NodeImpl::dispatchSimulatedMouseEvent):
(DOM::NodeImpl::dispatchUIEvent):
(DOM::NodeImpl::dispatchSubtreeModifiedEvent):
(DOM::NodeImpl::dispatchKeyEvent):
(DOM::NodeImpl::dispatchWheelEvent):
(DOM::ContainerNodeImpl::insertBefore):
(DOM::ContainerNodeImpl::replaceChild):
(DOM::ContainerNodeImpl::removeChild):
(DOM::ContainerNodeImpl::removeChildren):
(DOM::ContainerNodeImpl::appendChild):
(DOM::ContainerNodeImpl::addChild):
(DOM::ContainerNodeImpl::dispatchChildInsertedEvents):
Revision Changes Path
1.381 +31 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.380
retrieving revision 1.381
diff -u -r1.380 -r1.381
--- ChangeLog 17 Nov 2005 23:39:57 -0000 1.380
+++ ChangeLog 18 Nov 2005 21:34:35 -0000 1.381
@@ -1,3 +1,34 @@
+2005-11-18 David Harrison <harrison at apple.com>
+
+ <http://bugzilla.opendarwin.org/show_bug.cgi?id=5629>
+ REGRESSION: appendChild() does not remove nodes from source nodelist when inserting into destination
+
+ Enable event dispatch when calling removeChild() in loops. That it was
+ disabled previously was wrong because the DOM is not fragile at that point.
+ Makes the event dispatch forbiddance a debug-only check
+
+ * khtml/xml/dom_docimpl.cpp:
+ (DocumentImpl::createEvent):
+ * khtml/xml/dom_docimpl.h:
+ * khtml/xml/dom_nodeimpl.cpp:
+ (DOM::NodeImpl::dispatchEvent):
+ (DOM::NodeImpl::dispatchGenericEvent):
+ (DOM::NodeImpl::dispatchHTMLEvent):
+ (DOM::NodeImpl::dispatchWindowEvent):
+ (DOM::NodeImpl::dispatchMouseEvent):
+ (DOM::NodeImpl::dispatchSimulatedMouseEvent):
+ (DOM::NodeImpl::dispatchUIEvent):
+ (DOM::NodeImpl::dispatchSubtreeModifiedEvent):
+ (DOM::NodeImpl::dispatchKeyEvent):
+ (DOM::NodeImpl::dispatchWheelEvent):
+ (DOM::ContainerNodeImpl::insertBefore):
+ (DOM::ContainerNodeImpl::replaceChild):
+ (DOM::ContainerNodeImpl::removeChild):
+ (DOM::ContainerNodeImpl::removeChildren):
+ (DOM::ContainerNodeImpl::appendChild):
+ (DOM::ContainerNodeImpl::addChild):
+ (DOM::ContainerNodeImpl::dispatchChildInsertedEvents):
+
2005-11-17 Beth Dakin <bdakin at apple.com>
Reviewed by Hyatt and Eric.
1.272 +0 -18 WebCore/khtml/xml/dom_docimpl.cpp
Index: dom_docimpl.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_docimpl.cpp,v
retrieving revision 1.271
retrieving revision 1.272
diff -u -r1.271 -r1.272
--- dom_docimpl.cpp 15 Nov 2005 02:05:43 -0000 1.271
+++ dom_docimpl.cpp 18 Nov 2005 21:34:40 -0000 1.272
@@ -2396,28 +2396,10 @@
return m_defaultView;
}
-void DocumentImpl::forbidEventDispatch()
-{
- m_eventDispatchForbidden += 1;
-}
-
-void DocumentImpl::allowEventDispatch()
-{
- ASSERT(m_eventDispatchForbidden > 0);
- m_eventDispatchForbidden -= 1;
-}
-
-bool DocumentImpl::eventDispatchForbidden()
-{
- return m_eventDispatchForbidden > 0;
-}
-
EventImpl *DocumentImpl::createEvent(const DOMString &eventType, int &exceptioncode)
{
// createEvent ought to be at a time completely separate from DOM modifications that forbidEventDispatch
// (of course, the event _could_ be sent later, but this seems like a good bottleneck)
- ASSERT(!eventDispatchForbidden());
-
if (eventType == "UIEvents" || eventType == "UIEvent")
return new UIEventImpl();
else if (eventType == "MouseEvents" || eventType == "MouseEvent")
1.138 +0 -3 WebCore/khtml/xml/dom_docimpl.h
Index: dom_docimpl.h
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_docimpl.h,v
retrieving revision 1.137
retrieving revision 1.138
diff -u -r1.137 -r1.138
--- dom_docimpl.h 15 Nov 2005 02:05:43 -0000 1.137
+++ dom_docimpl.h 18 Nov 2005 21:34:40 -0000 1.138
@@ -444,9 +444,6 @@
void detachNodeIterator(NodeIteratorImpl *ni);
void notifyBeforeNodeRemoval(NodeImpl *n);
AbstractViewImpl *defaultView() const;
- void forbidEventDispatch();
- void allowEventDispatch();
- bool eventDispatchForbidden();
EventImpl *createEvent(const DOMString &eventType, int &exceptioncode);
// keep track of what types of event listeners are registered, so we don't
1.212 +41 -34 WebCore/khtml/xml/dom_nodeimpl.cpp
Index: dom_nodeimpl.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/xml/dom_nodeimpl.cpp,v
retrieving revision 1.211
retrieving revision 1.212
diff -u -r1.211 -r1.212
--- dom_nodeimpl.cpp 17 Nov 2005 00:32:32 -0000 1.211
+++ dom_nodeimpl.cpp 18 Nov 2005 21:34:40 -0000 1.212
@@ -101,6 +101,16 @@
};
int NodeImplCounter::count = 0;
static NodeImplCounter nodeImplCounter;
+
+static int gEventDispatchForbidden;
+#define forbidEventDispatch() gEventDispatchForbidden += 1
+#define allowEventDispatch() assert(gEventDispatchForbidden > 0); gEventDispatchForbidden -= 1
+#define eventDispatchForbidden() (gEventDispatchForbidden > 0)
+#else
+
+#define forbidEventDispatch()
+#define allowEventDispatch()
+#define eventDispatchForbidden()
#endif NDEBUG
NodeImpl::NodeImpl(DocumentImpl *doc)
@@ -497,7 +507,7 @@
bool NodeImpl::dispatchEvent(EventImpl *evt, int &exceptioncode, bool tempEvent)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
if (!evt || evt->type().isEmpty()) {
exceptioncode = EventException::_EXCEPTION_OFFSET + EventException::UNSPECIFIED_EVENT_TYPE_ERR;
return false;
@@ -541,7 +551,7 @@
bool NodeImpl::dispatchGenericEvent( EventImpl *evt, int &/*exceptioncode */)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
assert(evt->target());
evt->ref();
@@ -634,14 +644,14 @@
bool NodeImpl::dispatchHTMLEvent(const AtomicString &eventType, bool canBubbleArg, bool cancelableArg)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
int exceptioncode = 0;
return dispatchEvent(new EventImpl(eventType, canBubbleArg, cancelableArg), exceptioncode, true);
}
bool NodeImpl::dispatchWindowEvent(const AtomicString &eventType, bool canBubbleArg, bool cancelableArg)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
int exceptioncode = 0;
SharedPtr<EventImpl> evt = new EventImpl(eventType, canBubbleArg, cancelableArg);
SharedPtr<DocumentImpl> doc = getDocument();
@@ -677,7 +687,7 @@
bool NodeImpl::dispatchMouseEvent(QMouseEvent *_mouse, const AtomicString &overrideType, int overrideDetail)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
int detail = overrideDetail; // defaults to 0
AtomicString eventType;
if (!overrideType.isEmpty()) {
@@ -736,7 +746,7 @@
bool NodeImpl::dispatchSimulatedMouseEvent(const AtomicString &eventType)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
// Like Gecko, we just pass 0 for everything when we make a fake mouse event.
// Internet Explorer instead gives the current mouse position and state.
return dispatchMouseEvent(eventType, 0, 0, 0, 0, 0, 0, false, false, false, false);
@@ -746,7 +756,7 @@
int clientX, int clientY, int screenX, int screenY,
bool ctrlKey, bool altKey, bool shiftKey, bool metaKey)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
if (disabled()) // Don't even send DOM events for disabled controls..
return true;
@@ -802,7 +812,7 @@
bool NodeImpl::dispatchUIEvent(const AtomicString &eventType, int detail)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
assert (!( (eventType != DOMFocusInEvent &&
eventType != DOMFocusOutEvent &&
eventType != DOMActivateEvent)));
@@ -879,10 +889,8 @@
bool NodeImpl::dispatchSubtreeModifiedEvent(bool sendChildrenChanged)
{
- // forbidder will send this event later
- if (getDocument()->eventDispatchForbidden())
- return true;
-
+ assert(!eventDispatchForbidden());
+
// FIXME: Pull this whole if clause out of this function.
if (sendChildrenChanged) {
notifyNodeListsChildrenChanged();
@@ -899,7 +907,7 @@
bool NodeImpl::dispatchKeyEvent(QKeyEvent *key)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
int exceptioncode = 0;
//kdDebug(6010) << "DOM::NodeImpl: dispatching keyboard event" << endl;
KeyboardEventImpl *keyboardEventImpl = new KeyboardEventImpl(key, getDocument()->defaultView());
@@ -925,7 +933,7 @@
void NodeImpl::dispatchWheelEvent(QWheelEvent *e)
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
if (e->delta() == 0)
return;
@@ -2144,13 +2152,13 @@
// If child is already present in the tree, first remove it
NodeImpl *newParent = child->parentNode();
- if(newParent)
+ if (newParent)
newParent->removeChild( child, exceptioncode );
- if ( exceptioncode )
+ if (exceptioncode)
return 0;
// Add child in the correct position
- getDocument()->forbidEventDispatch();
+ forbidEventDispatch();
if (prev)
prev->setNextSibling(child);
else
@@ -2164,7 +2172,7 @@
// ### should we detach() it first if it's already attached?
if (attached() && !child->attached())
child->attach();
- getDocument()->allowEventDispatch();
+ allowEventDispatch();
// Dispatch the mutation events
dispatchChildInsertedEvents(child,exceptioncode);
@@ -2212,16 +2220,17 @@
// Add the new child(ren)
while (child) {
- getDocument()->forbidEventDispatch();
nextChild = isFragment ? child->nextSibling() : 0;
// If child is already present in the tree, first remove it
NodeImpl *newParent = child->parentNode();
- if(newParent)
+ if (newParent)
newParent->removeChild( child, exceptioncode );
if (exceptioncode)
return 0;
+ forbidEventDispatch();
+
// Add child in the correct position
if (prev) prev->setNextSibling(child);
if (next) next->setPreviousSibling(child);
@@ -2235,7 +2244,7 @@
// ### should we detach() it first if it's already attached?
if (attached() && !child->attached())
child->attach();
- getDocument()->allowEventDispatch();
+ allowEventDispatch();
// Dispatch the mutation events
dispatchChildInsertedEvents(child,exceptioncode);
@@ -2304,7 +2313,7 @@
if (exceptioncode)
return 0;
- getDocument()->forbidEventDispatch();
+ forbidEventDispatch();
// Remove from rendering tree
if (oldChild->attached())
@@ -2326,7 +2335,7 @@
getDocument()->setDocumentChanged(true);
- getDocument()->allowEventDispatch();
+ allowEventDispatch();
// Dispatch post-removal mutation events
dispatchSubtreeModifiedEvent();
@@ -2353,7 +2362,7 @@
for (n = _first; n != 0; n = n->nextSibling())
willRemoveChild(n);
- getDocument()->forbidEventDispatch();
+ forbidEventDispatch();
while ((n = _first) != 0) {
NodeImpl *next = n->nextSibling();
@@ -2373,7 +2382,7 @@
_first = next;
}
_last = 0;
- getDocument()->allowEventDispatch();
+ allowEventDispatch();
// Dispatch a single post-removal mutation event denoting a modified subtree.
dispatchSubtreeModifiedEvent();
@@ -2406,7 +2415,6 @@
NodeImpl *child = isFragment ? newChild->firstChild() : newChild;
while (child) {
- getDocument()->forbidEventDispatch();
nextChild = isFragment ? child->nextSibling() : 0;
// If child is already present in the tree, first remove it
@@ -2417,6 +2425,8 @@
return 0;
}
+ forbidEventDispatch();
+
// Append child to the end of the list
child->setParent(this);
@@ -2425,18 +2435,15 @@
child->setPreviousSibling(_last);
_last->setNextSibling(child);
_last = child;
- }
- else
- {
+ } else
_first = _last = child;
- }
// Add child to the rendering tree
// ### should we detach() it first if it's already attached?
if (attached() && !child->attached())
child->attach();
- getDocument()->allowEventDispatch();
+ allowEventDispatch();
// Dispatch the mutation events
dispatchChildInsertedEvents(child,exceptioncode);
@@ -2501,7 +2508,7 @@
if (getDocument()->isHTMLDocument() && !childAllowed(newChild))
return 0;
- getDocument()->forbidEventDispatch();
+ forbidEventDispatch();
// just add it...
newChild->setParent(this);
@@ -2521,7 +2528,7 @@
newChild->insertedIntoDocument();
childrenChanged();
- getDocument()->allowEventDispatch();
+ allowEventDispatch();
if(newChild->nodeType() == Node::ELEMENT_NODE)
return newChild;
@@ -2807,7 +2814,7 @@
void ContainerNodeImpl::dispatchChildInsertedEvents( NodeImpl *child, int &exceptioncode )
{
- assert(!getDocument()->eventDispatchForbidden());
+ assert(!eventDispatchForbidden());
if (inDocument())
child->insertedIntoDocument();
else
More information about the webkit-changes
mailing list