nddelima 2004/10/21 14:51:05 Modified: java/src/org/apache/xerces/dom CoreDocumentImpl.java LCount.java DocumentImpl.java CharacterDataImpl.java Log: Patch that fixes some DOM Level 2 events problems such as: - Dispatching of events to a subtree was being stopped from reaching descendants when a higher node in the tree did not have a registered listener. - Sibling of the target node and its descendants were receiving events during dispatch to the target node's subtree. - During replaceData, only one event to mark the replacement rather than two for insert and delete is fired. - DOMAttrModified was not being fired when the text node child of an attribute was modified. - Adds a total field to LCount to tell whether any listeners are registered rather than checking captures + bubbles + default Thanks to Nalea for her contribution [1]. [1] http://nagoya.apache.org/eyebrowse/[EMAIL PROTECTED]&msgNo=4662 Revision Changes Path 1.81 +17 -4 xml-xerces/java/src/org/apache/xerces/dom/CoreDocumentImpl.java Index: CoreDocumentImpl.java =================================================================== RCS file: /home/cvs/xml-xerces/java/src/org/apache/xerces/dom/CoreDocumentImpl.java,v retrieving revision 1.80 retrieving revision 1.81 diff -u -r1.80 -r1.81 --- CoreDocumentImpl.java 5 Oct 2004 17:12:50 -0000 1.80 +++ CoreDocumentImpl.java 21 Oct 2004 21:51:05 -0000 1.81 @@ -2599,15 +2599,15 @@ } /** - * A method to be called when a character data node has been modified + * A method to be called when a character data node is about to be modified */ - void modifyingCharacterData(NodeImpl node) { + void modifyingCharacterData(NodeImpl node, boolean replace) { } /** * A method to be called when a character data node has been modified */ - void modifiedCharacterData(NodeImpl node, String oldvalue, String value) { + void modifiedCharacterData(NodeImpl node, String oldvalue, String value, boolean replace) { } /** @@ -2646,6 +2646,19 @@ void replacedNode(NodeImpl node) { } + /** + * A method to be called when a character data node is about to be replaced + */ + void replacingData(NodeImpl node) { + } + + /** + * method to be called when a character data node has been replaced. + */ + void replacedCharacterData(NodeImpl node, String oldvalue, String value) { + } + + /** * A method to be called when an attribute value has been modified */ 1.9 +2 -7 xml-xerces/java/src/org/apache/xerces/dom/LCount.java Index: LCount.java =================================================================== RCS file: /home/cvs/xml-xerces/java/src/org/apache/xerces/dom/LCount.java,v retrieving revision 1.8 retrieving revision 1.9 diff -u -r1.8 -r1.9 --- LCount.java 5 Oct 2004 17:12:51 -0000 1.8 +++ LCount.java 21 Oct 2004 21:51:05 -0000 1.9 @@ -26,11 +26,6 @@ Move it when we have a chance to do so. Sorry; we were rushed. - ***** Also, I'm currently asking "are there any listeners" - by testing captures+bubbles+defaults =? 0. It would probably - make sense to have a separate "total" field, calculated at - add/remove, to save a few cycles during dispatch. Fix. - ???? CONCERN: Hashtables are known to be "overserialized" in current versions of Java. That may impact performance. @@ -47,7 +42,7 @@ class LCount { static java.util.Hashtable lCounts=new java.util.Hashtable(); - public int captures=0,bubbles=0,defaults=0; + public int captures=0,bubbles=0,defaults, total=0; static LCount lookup(String evtName) { 1.82 +105 -48 xml-xerces/java/src/org/apache/xerces/dom/DocumentImpl.java Index: DocumentImpl.java =================================================================== RCS file: /home/cvs/xml-xerces/java/src/org/apache/xerces/dom/DocumentImpl.java,v retrieving revision 1.81 retrieving revision 1.82 diff -u -r1.81 -r1.82 --- DocumentImpl.java 5 Oct 2004 17:12:50 -0000 1.81 +++ DocumentImpl.java 21 Oct 2004 21:51:05 -0000 1.82 @@ -517,7 +517,7 @@ // a listener to dispatch to if (type == null || type.equals("") || listener == null) return; - + // Each listener may be registered only once per type per phase. // Simplest way to code that is to zap the previous entry, if any. removeEventListener(node, type, listener, useCapture); @@ -531,10 +531,14 @@ // Record active listener LCount lc = LCount.lookup(type); - if (useCapture) + if (useCapture) { ++lc.captures; - else + ++lc.total; + } + else { ++lc.bubbles; + ++lc.total; + } } // addEventListener(NodeImpl,String,EventListener,boolean) :void @@ -574,10 +578,14 @@ // Remove active listener LCount lc = LCount.lookup(type); - if (useCapture) + if (useCapture) { --lc.captures; - else + --lc.total; + } + else { --lc.bubbles; + --lc.total; + } break; // Found it; no need to loop farther. } @@ -658,7 +666,7 @@ // If nobody is listening for this event, discard immediately LCount lc = LCount.lookup(evt.getType()); - if (lc.captures + lc.bubbles + lc.defaults == 0) + if (lc.total == 0) return evt.preventDefault; // INITIALIZE THE EVENT'S DISPATCH STATUS @@ -720,6 +728,7 @@ } } + // Both AT_TARGET and BUBBLE use non-capturing listeners. if (lc.bubbles > 0) { // AT_TARGET PHASE: Event is dispatched to NON-CAPTURING listeners @@ -802,32 +811,52 @@ * are dispatched to an entire subtree. This is the distribution code * therefor. They DO NOT bubble, thanks be, but may be captured. * <p> + * Similar to code in dispatchingEventToSubtree however this method + * is only used on the target node and does not start a dispatching chain + * on the sibling of the target node as this is not part of the subtree * ***** At the moment I'm being sloppy and using the normal * capture dispatcher on every node. This could be optimized hugely * by writing a capture engine that tracks our position in the tree to * update the capture chain without repeated chases up to root. - * @param node node to dispatch to - * @param n node which was directly inserted or removed + * @param n target node (that was directly inserted or removed) * @param e event to be sent to that node and its subtree */ - protected void dispatchEventToSubtree(NodeImpl node, Node n, Event e) { - Vector nodeListeners = getEventListeners(node); - if (nodeListeners == null || n == null) - return; - - // ***** Recursive implementation. This is excessively expensive, - // and should be replaced in conjunction with optimization - // mentioned above. + protected void dispatchEventToSubtree(Node n, Event e) { + ((NodeImpl) n).dispatchEvent(e); if (n.getNodeType() == Node.ELEMENT_NODE) { NamedNodeMap a = n.getAttributes(); for (int i = a.getLength() - 1; i >= 0; --i) - dispatchEventToSubtree(node, a.item(i), e); + dispatchingEventToSubtree(a.item(i), e); } - dispatchEventToSubtree(node, n.getFirstChild(), e); - dispatchEventToSubtree(node, n.getNextSibling(), e); + dispatchingEventToSubtree(n.getFirstChild(), e); + } // dispatchEventToSubtree(NodeImpl,Node,Event) :void + + /** + * Dispatches event to the target node's descendents recursively + * + * @param n node to dispatch to + * @param e event to be sent to that node and its subtree + */ + protected void dispatchingEventToSubtree(Node n, Event e) { + if (n==null) + return; + + // ***** Recursive implementation. This is excessively expensive, + // and should be replaced in conjunction with optimization + // mentioned above. + ((NodeImpl) n).dispatchEvent(e); + if (n.getNodeType() == Node.ELEMENT_NODE) { + NamedNodeMap a = n.getAttributes(); + for (int i = a.getLength() - 1; i >= 0; --i) + dispatchingEventToSubtree(a.item(i), e); + } + dispatchingEventToSubtree(n.getFirstChild(), e); + dispatchingEventToSubtree(n.getNextSibling(), e); + } + /** * NON-DOM INTERNAL: Return object for getEnclosingAttr. Carries * (two values, the Attr node affected (if any) and its previous @@ -889,7 +918,7 @@ if (enclosingAttr != null) { LCount lc = LCount.lookup(MutationEventImpl.DOM_ATTR_MODIFIED); owner = (NodeImpl) enclosingAttr.getOwnerElement(); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { if (owner != null) { MutationEventImpl me = new MutationEventImpl(); me.initMutationEvent(MutationEventImpl.DOM_ATTR_MODIFIED, @@ -907,7 +936,7 @@ // "This event is dispatched after all other events caused by the // mutation have been fired." LCount lc = LCount.lookup(MutationEventImpl.DOM_SUBTREE_MODIFIED); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { MutationEvent me = new MutationEventImpl(); me.initMutationEvent(MutationEventImpl.DOM_SUBTREE_MODIFIED, true, false, null, null, @@ -940,7 +969,7 @@ // was requested, we need to preserve its previous value for // that event. LCount lc = LCount.lookup(MutationEventImpl.DOM_ATTR_MODIFIED); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { NodeImpl eventAncestor = node; while (true) { if (eventAncestor == null) @@ -955,6 +984,8 @@ } else if (type == Node.ENTITY_REFERENCE_NODE) eventAncestor = eventAncestor.parentNode(); + else if (type == Node.TEXT_NODE) + eventAncestor = eventAncestor.parentNode(); else return; // Any other parent means we're not in an Attr @@ -965,34 +996,51 @@ /** * A method to be called when a character data node has been modified */ - void modifyingCharacterData(NodeImpl node) { + void modifyingCharacterData(NodeImpl node, boolean replace) { if (mutationEvents) { - saveEnclosingAttr(node); + if (!replace) { + saveEnclosingAttr(node); + } } } /** * A method to be called when a character data node has been modified */ - void modifiedCharacterData(NodeImpl node, String oldvalue, String value) { + void modifiedCharacterData(NodeImpl node, String oldvalue, String value, boolean replace) { if (mutationEvents) { - // MUTATION POST-EVENTS: - LCount lc = - LCount.lookup(MutationEventImpl.DOM_CHARACTER_DATA_MODIFIED); - if (lc.captures + lc.bubbles + lc.defaults > 0) { - MutationEvent me = new MutationEventImpl(); - me.initMutationEvent( - MutationEventImpl.DOM_CHARACTER_DATA_MODIFIED, - true, false, null, - oldvalue, value, null, (short) 0); - dispatchEvent(node, me); - } + if (!replace) { + // MUTATION POST-EVENTS: + LCount lc = + LCount.lookup(MutationEventImpl.DOM_CHARACTER_DATA_MODIFIED); + if (lc.total > 0) { + MutationEvent me = new MutationEventImpl(); + me.initMutationEvent( + MutationEventImpl.DOM_CHARACTER_DATA_MODIFIED, + true, false, null, + oldvalue, value, null, (short) 0); + dispatchEvent(node, me); + } - // Subroutine: Transmit DOMAttrModified and DOMSubtreeModified, - // if required. (Common to most kinds of mutation) - dispatchAggregateEvents(node, savedEnclosingAttr); - } // End mutation postprocessing + // Subroutine: Transmit DOMAttrModified and DOMSubtreeModified, + // if required. (Common to most kinds of mutation) + dispatchAggregateEvents(node, savedEnclosingAttr); + } // End mutation postprocessing + } + } + + /** + * A method to be called when a character data node has been replaced + */ + void replacedCharacterData(NodeImpl node, String oldvalue, String value) { + //now that we have finished replacing data, we need to perform the same actions + //that are required after a character data node has been modified + //send the value of false for replace parameter so that mutation + //events if appropriate will be initiated + modifiedCharacterData(node, oldvalue, value, false); } + + /** * A method to be called when a node is about to be inserted in the tree. @@ -1014,7 +1062,7 @@ // "Local" events (non-aggregated) // New child is told it was inserted, and where LCount lc = LCount.lookup(MutationEventImpl.DOM_NODE_INSERTED); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { MutationEventImpl me = new MutationEventImpl(); me.initMutationEvent(MutationEventImpl.DOM_NODE_INSERTED, true, false, node, @@ -1026,7 +1074,7 @@ // to the Doc. lc = LCount.lookup( MutationEventImpl.DOM_NODE_INSERTED_INTO_DOCUMENT); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { NodeImpl eventAncestor = node; if (savedEnclosingAttr != null) eventAncestor = (NodeImpl) @@ -1050,7 +1098,7 @@ .DOM_NODE_INSERTED_INTO_DOCUMENT, false,false,null,null, null,null,(short)0); - dispatchEventToSubtree(node, newInternal, me); + dispatchEventToSubtree(newInternal, me); } } } @@ -1102,7 +1150,7 @@ } // Child is told that it is about to be removed LCount lc = LCount.lookup(MutationEventImpl.DOM_NODE_REMOVED); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { MutationEventImpl me= new MutationEventImpl(); me.initMutationEvent(MutationEventImpl.DOM_NODE_REMOVED, true, false, node, null, @@ -1114,7 +1162,7 @@ // losing that status lc = LCount.lookup( MutationEventImpl.DOM_NODE_REMOVED_FROM_DOCUMENT); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { NodeImpl eventAncestor = this; if(savedEnclosingAttr != null) eventAncestor = (NodeImpl) @@ -1130,7 +1178,7 @@ MutationEventImpl.DOM_NODE_REMOVED_FROM_DOCUMENT, false, false, null, null, null, null, (short) 0); - dispatchEventToSubtree(node, oldChild, me); + dispatchEventToSubtree(oldChild, me); } } } @@ -1159,6 +1207,15 @@ saveEnclosingAttr(node); } } + + /** + * A method to be called when character data is about to be replaced in the tree. + */ + void replacingData (NodeImpl node) { + if (mutationEvents) { + saveEnclosingAttr(node); + } + } /** * A method to be called when a node has been replaced in the tree. @@ -1209,7 +1266,7 @@ // If we have to send DOMAttrModified (determined earlier), // do so. LCount lc = LCount.lookup(MutationEventImpl.DOM_ATTR_MODIFIED); - if (lc.captures + lc.bubbles + lc.defaults > 0) { + if (lc.total > 0) { MutationEventImpl me= new MutationEventImpl(); me.initMutationEvent(MutationEventImpl.DOM_ATTR_MODIFIED, true, false, attr, 1.23 +111 -54 xml-xerces/java/src/org/apache/xerces/dom/CharacterDataImpl.java Index: CharacterDataImpl.java =================================================================== RCS file: /home/cvs/xml-xerces/java/src/org/apache/xerces/dom/CharacterDataImpl.java,v retrieving revision 1.22 retrieving revision 1.23 diff -u -r1.22 -r1.23 --- CharacterDataImpl.java 5 Oct 2004 17:12:50 -0000 1.22 +++ CharacterDataImpl.java 21 Oct 2004 21:51:05 -0000 1.23 @@ -90,6 +90,13 @@ return data; } + /** Convenience wrapper for calling setNodeValueInternal when + * we are not performing a replacement operation + */ + protected void setNodeValueInternal (String value) { + setNodeValueInternal(value, false); + } + /** This function added so that we can distinguish whether * setNodeValue has been called from some other DOM functions. * or by the client.<p> @@ -97,7 +104,7 @@ * from the high-level functions in CharacterData, and another * type if the client simply calls setNodeValue(value). */ - protected void setNodeValueInternal(String value) { + protected void setNodeValueInternal(String value, boolean replace) { if (isReadOnly()) { String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "NO_MODIFICATION_ALLOWED_ERR", null); throw new DOMException(DOMException.NO_MODIFICATION_ALLOWED_ERR, msg); @@ -114,12 +121,12 @@ CoreDocumentImpl ownerDocument = ownerDocument(); // notify document - ownerDocument.modifyingCharacterData(this); + ownerDocument.modifyingCharacterData(this, replace); this.data = value; // notify document - ownerDocument.modifiedCharacterData(this, oldvalue, value); + ownerDocument.modifiedCharacterData(this, oldvalue, value, replace); } /** @@ -203,38 +210,54 @@ */ public void deleteData(int offset, int count) throws DOMException { + + internalDeleteData(offset, count, false); + } // deleteData(int,int) + + /** NON-DOM INTERNAL: Within DOM actions, we sometimes need to be able + * to control which mutation events are spawned. This version of the + * deleteData operation allows us to do so. It is not intended + * for use by application programs. + */ + void internalDeleteData (int offset, int count, boolean replace) + throws DOMException { + if (isReadOnly()) { - String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "NO_MODIFICATION_ALLOWED_ERR", null); - throw new DOMException(DOMException.NO_MODIFICATION_ALLOWED_ERR, msg); - } + String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "NO_MODIFICATION_ALLOWED_ERR", null); + throw new DOMException(DOMException.NO_MODIFICATION_ALLOWED_ERR, msg); + } + + if (count < 0) { + String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "INDEX_SIZE_ERR", null); + throw new DOMException(DOMException.INDEX_SIZE_ERR, msg); + } + + if (needsSyncData()) { + synchronizeData(); + } + int tailLength = Math.max(data.length() - count - offset, 0); + try { + String value = data.substring(0, offset) + + (tailLength > 0 + ? data.substring(offset + count, offset + count + tailLength) + : ""); - if (count < 0) { - String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "INDEX_SIZE_ERR", null); - throw new DOMException(DOMException.INDEX_SIZE_ERR, msg); - } + + setNodeValueInternal(value, replace); - if (needsSyncData()) { - synchronizeData(); - } - int tailLength = Math.max(data.length() - count - offset, 0); - try { - String value = data.substring(0, offset) + - (tailLength > 0 - ? data.substring(offset + count, offset + count + tailLength) - : ""); - - setNodeValueInternal(value); - - // notify document - ownerDocument().deletedText(this, offset, count); - } - catch (StringIndexOutOfBoundsException e) { - String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "INDEX_SIZE_ERR", null); - throw new DOMException(DOMException.INDEX_SIZE_ERR, msg); - } + // notify document + ownerDocument().deletedText(this, offset, count); + } + catch (StringIndexOutOfBoundsException e) { + String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "INDEX_SIZE_ERR", null); + throw new DOMException(DOMException.INDEX_SIZE_ERR, msg); + } + + } // internalDeleteData(int,int,boolean) + - } // deleteData(int,int) + /** * Insert additional characters into the data stored in this node, @@ -248,30 +271,47 @@ public void insertData(int offset, String data) throws DOMException { - if (isReadOnly()) { - String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "NO_MODIFICATION_ALLOWED_ERR", null); - throw new DOMException(DOMException.NO_MODIFICATION_ALLOWED_ERR, msg); - } + internalInsertData(offset, data, false); + + } // insertData(int,int) + + + + /** NON-DOM INTERNAL: Within DOM actions, we sometimes need to be able + * to control which mutation events are spawned. This version of the + * insertData operation allows us to do so. It is not intended + * for use by application programs. + */ + void internalInsertData (int offset, String data, boolean replace) + throws DOMException { + + if (isReadOnly()) { + String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "NO_MODIFICATION_ALLOWED_ERR", null); + throw new DOMException(DOMException.NO_MODIFICATION_ALLOWED_ERR, msg); + } + + if (needsSyncData()) { + synchronizeData(); + } + try { + String value = + new StringBuffer(this.data).insert(offset, data).toString(); - if (needsSyncData()) { - synchronizeData(); - } - try { - String value = - new StringBuffer(this.data).insert(offset, data).toString(); - - setNodeValueInternal(value); - - // notify document - ownerDocument().insertedText(this, offset, data.length()); - } - catch (StringIndexOutOfBoundsException e) { - String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "INDEX_SIZE_ERR", null); - throw new DOMException(DOMException.INDEX_SIZE_ERR, msg); - } + + setNodeValueInternal(value, replace); + + // notify document + ownerDocument().insertedText(this, offset, data.length()); + } + catch (StringIndexOutOfBoundsException e) { + String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "INDEX_SIZE_ERR", null); + throw new DOMException(DOMException.INDEX_SIZE_ERR, msg); + } - } // insertData(int,int) + } // internalInsertData(int,String,boolean) + + /** * Replace a series of characters at the specified (zero-based) * offset with a new string, NOT necessarily of the same @@ -305,8 +345,25 @@ // DOMSubtreeModified. But mutation events are // underspecified; I don't feel compelled // to deal with it right now. - deleteData(offset, count); - insertData(offset, data); + if (isReadOnly()) { + String msg = DOMMessageFormatter.formatMessage(DOMMessageFormatter.DOM_DOMAIN, "NO_MODIFICATION_ALLOWED_ERR", null); + throw new DOMException(DOMException.NO_MODIFICATION_ALLOWED_ERR, msg); + } + + if (needsSyncData()) { + synchronizeData(); + } + + //notify document + ownerDocument().replacingData(this); + + // keep old value for document notification + String oldvalue = this.data; + + internalDeleteData(offset, count, true); + internalInsertData(offset, data, true); + + ownerDocument().replacedCharacterData(this, oldvalue, this.data); } // replaceData(int,int,String)
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]