Community
Participate
Working Groups
Version: 3.4.0 Build id: I20071213-1700 WTP 3.0 The W3C maintains a suite of Tests for checking conformance for a particular level of DOM compliance. These results are from running the WTP DOM implementation against the DOM Level 1 XML Core tests. It does not include results for DOM Level 1 HTML Core. Tests Passed: 358 Errors: 21 Failures: 139 Did Not Complete: 9 This bug report probably should be used as a tracking bug for related bugs and patches. More information about the Conformance test suite can be found at: http://www.w3.org/DOM/Test/
Created attachment 86314 [details] Test Plugin Project that implements the W3C DOM Test Suite http://www.starstandard.org/download/Eclipse/wtpdomtestsuite.zip This bundle is a repackaging of the W3C DOM Test suite in a form that can be run as Plugin-Tests. The actual WTP Test for DOM level 1, can be found in the org.eclipse.wst.xml/org.eclipse.wst.xml.core.document.level1 source folder. Run TestWTP.java and it will execute the rest of the test suite. The test suite has been slightly updated to check to see if isExpandEntityReferences() is set to true or false. WTP doesn't expand entity references in the DOM, so these tests are skipped when run. Because WTP is used as an DOM/Editor it doesn't make sense to expand the Entity References when doing Gets on a node's value. See bug 214440 with some patches that fix several minor issues with the WTP DOM.
AttrImpl is extending the wrong class. It currently is extending the NodeImpl class, which technically is correct. However, the functionality that it needs to run through the DOM Level 1 core tests are contained in the NodeContainer class. ElementImpl extends NodeContainer which Extends NodeImpl. Ultimately what needs to happen is that either NodeImpl needs to move down a level and extend NodeContainer, or NodeContainer's functionality needs to be moved into NodeImpl.
(In reply to comment #2) > AttrImpl is extending the wrong class. It currently is extending the NodeImpl > class, which technically is correct. However, the functionality that it needs > to run through the DOM Level 1 core tests are contained in the NodeContainer > class. ElementImpl extends NodeContainer which Extends NodeImpl. > > Ultimately what needs to happen is that either NodeImpl needs to move down a > level and extend NodeContainer, or NodeContainer's functionality needs to be > moved into NodeImpl. > I don't think AttrImpl should extend NodeContainer. The purpose of NodeContainer is to provide common implementations for things that are containers, e.g. Element, which can contain other things, like Elements. That is, one of those things can have children. And Attributes can not contain other things, so ... not sure I follow that reasoning. Surely an Attribute does not need methods such as getChildren, getFirstChild, etc. Right? May I suggest, for illustration, education, and a safer code change, that if you want AttrImpl to have some behavior of NodeContainer, that you create a new class, maybe called AbstractAttrImpl, it would extend NodeImpl and AttrImpl would extend AbstractAttrImpl. That way it'd be obvious what behavior was needed, without accidently picking up methods/behavior that was not needed. And, would make it conceptually easier for some of us to understand.
(In reply to comment #3) > I don't think AttrImpl should extend NodeContainer. > The purpose of NodeContainer is to provide common implementations for things > that are containers, e.g. Element, which can contain other things, like > Elements. That is, one of those things can have children. > And Attributes can not contain other things, so ... not sure I follow that > reasoning. Surely an Attribute does not need methods such as getChildren, > getFirstChild, etc. Right? > Actually, yes it does need getChildren and getFirstChild, because the nodes that an attribute can have are TextImpl nodes. An attribute may not have any other type of nodes besides TextImpl nodes. According to the W3C Spec for DOM Level 1 Core XML on AttrImpl: http://www.w3.org/TR/2000/WD-DOM-Level-1-20000929/level-one-core.html#ID-637646024 "In XML, where the value of an attribute can contain entity references, the child nodes of the Attr node provide a representation in which entity references are not expanded. These child nodes may be either Text or EntityReference nodes. Because the attribute type may be unknown, there are no tokenized attribute values." Also, the only methods that should return null for the AttrImpl are: "Attr objects inherit the Node interface, but since they are not actually child nodes of the element they describe, the DOM does not consider them part of the document tree. Thus, the Node attributes parentNode, previousSibling, and nextSibling have a null value for Attr objects. The DOM takes the view that attributes are properties of elements rather than having a separate identity from the elements they are associated with; this should make it more efficient to implement such features as default attributes associated with all elements of a given type." So for the WTP implementation, the getChildren and getFirstChild should always return a TextImpl that contains the value of the attribute. The only item I've found that we shouldn't implement, is the expanding of the entity references, because that isn't productive, and there is a method called isExpandedReference() that tells whether the DOM implementation supports this. I created a WTPDocumentBuilderFactory implementation in order to get this test suite to run. The BuilderFactory also has items like isValidating() which tells whether its a dom validating parser or no.
Some news just released from the W3C that might make adding the W3C DOM Test Suite to WTP easier to do. http://www.w3.org/Consortium/Legal/2008/04-testsuite-copyright This should allow WTP to check in the W3C test suite (after IP review of course), and be able to add it's tests to the tests cases that validate the WTP DOM (currently very few). This would immediately enhance the code coverage and verification of DOM Level 1 support, and can be extened for DOM Level 2 and Level 3 support.
It seems like this one is simply a question of kicking an IP revioew process so that it could be added to CVS. Notwithstanding the discussion of how to change AttrImpl to support container behavior, this bug has no impact on adopters. Could we drop this into Eclipse Foundation's queue and get it cleared?
Created attachment 121035 [details] DOM Level 1 Unit Test Code Coverage Report Results of running code coverage for the DOM Level 1 tests. Does not include test suite to check DOM Level 2 or DOM Level 3 compliance. WTP DOM still has 20 errors, 115 failures out of 499 unit tests.
Nitin if you want I can take this and get the ball rolling for the 3.2 time frame. I'll submit the code for IP Review, so that we can get clearance to check it into CVS.
Ok.
Created CQ 3252 to start the process of getting the test suite code approved.
Just adding a note here, since it relates. The XPath 2.0 processor in source editing is going to require DOM Level 3 support for many of the functionality. It is going to be important to address the DOM Level compliance issue in the relatively near term. I'm still waiting on IP to review the W3C Test suite code. But I may after finishing up the XPath 2.0 conformance test suite, start working on some patches for the DOM Level 1 compliance. Possible issue though is that some aspects of WTP's DOM implementation are implemented wrong (i.e. assumptions like Attr nodes not having children (they have entity and text children)) so some may require internal API breakage, I'm hoping to avoid interface changes but depending on WTP's implementation this may be unavoidable to get compliance. I want to stress again that it is vital that we get a compliant dom implementation to Level 1, Level 2, and Level 3 specs. Particularly level 3, as it provides some standard ways to get a PSVI (Content Model Query).
I've been given preliminary approval with a parallel IP exception to check the test suite into CVS. Will try to do this over the next couple of days.
Alright I did get this checked in. Currently the test suite is only setup to run the XML tests, there would need to be another test setup to run the HTML Level 1 related DOM tests. Results currently are: Tests: 499 Failures: 115 Errors: 20 So we have 135 tests that are failing out of 499 DOM Level 1 tests (some are currently not run due to the way entities are not handled with the WTP DOM). This gives a 73% pass rating. This is an improvement from when the test suite was first run against the WTP DOM back in 2006.
(In reply to comment #13) > Alright I did get this checked in. I think you put it in the wrong place; it should be under the "tests" folder.
(In reply to comment #14) > (In reply to comment #13) > > Alright I did get this checked in. > > I think you put it in the wrong place; it should be under the "tests" folder. You are correct. I noticed this the other day, please feel free to move it to the correct place. I'm traveling at the moment so won't get to this until Friday at the earliest.
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > Alright I did get this checked in. > > > > I think you put it in the wrong place; it should be under the "tests" folder. > > You are correct. I noticed this the other day, please feel free to move it to > the correct place. I'm traveling at the moment so won't get to this until > Friday at the earliest. Nitin can you make me QA on this particular item, and put it into triaged state? I think it will be better reflect the intent. Unless you want to remain QA on this. I still plan to work further on this.
Out of curiosity, is the status in comment 13 still correct has more work been done on the interim?
(In reply to comment #17) > Out of curiosity, is the status in comment 13 still correct has more work been > done on the interim? The status has actually improved since comment 13 was written. I have a plan to target getting these tests running as part of the build early in 3.3M1, and work on adding additional support and corrections as well. Once DOM Level 1 is passing completely, then I'll get the necessary IP approvals for DOM Level 2 and DOM Level 3. Patches are always welcomed as well.
> The status has actually improved since comment 13 was written. I have a plan Cool. Thanks for your work on this.
I've started work on this again. Current stats are 104 failures, 20 errors out of 499 tests. Work on this is happening in a GitHub clone, and I'll attach a patch once all tests are passing. If anybody wants to follow along, you can watch the wtp-dom branch at: http://github.com/kingargyle/WTP-Source-Editing/tree/wtp-dom
First fix. getNamedItem() for element attributes was not returning an attribute if it had a default value defined. If a ContentModel is associated with the xml file, and the element has a default value specified for an attribute, the DOM needs to return the attribute and the default value, even though it may not physically reside in the xml file itself. commit 80056a519b75806da06dc14b388e8dd7d869cbc9 Author: David Carver <d_a_carver@yahoo.com> Date: Tue Aug 10 08:28:28 2010 -0700 GitHub 2: Fix Default attribute values. * Return default values for attributes that may not be in the XML but are defined in the ContentModel.
commit cc22204e6b5455156c5e5bb2fcff3bcfefcc86b7 Author: David Carver <d_a_carver@yahoo.com> Date: Tue Aug 10 20:32:17 2010 -0700 Fix deleteData method on Text and CharaData to handle a few edge cases. In particular, the methods were incorrectly tossing DOMExceptions when the count would exceed the length. According to the DOM specification, if the count goes beyond the length of the string, it should stop at the end of the string and delete any data from that point forward. This patch fixes this issue.
commit f30a672d433d4fe0c7da59fcd89d762a3b650ddf Author: David Carver <d_a_carver@yahoo.com> Date: Tue Aug 10 20:56:23 2010 -0700 Fix for create element to create default attributes if content model exists. The issue here was that no check on a content model was being done, and no check to see if there were any default attributes that needed to be created. This change looks up the attributes for the node, and will add any default attributes that need to be created as well. This only happens if there is an appropriate content model entry found.
Reworked some of the tests that weren't relevant to the WTP DOM implementation. This included adding appropriate DOMException.DOM_UNSUPPORTED_OPERATIONS to several methods in the Attributes. Particularly the ones dealing with child nodes on attributes. commit faa880e356f49581a384347a3a1c80ef308b6d9b Author: David Carver <d_a_carver@yahoo.com> Date: Sun Aug 15 19:56:48 2010 -0700 commit ac9ea2763488f332879dc24bcdc2472e1538fb87 Author: David Carver <d_a_carver@yahoo.coom> Date: Fri Aug 13 07:01:30 2010 -0700 Refactor Test code to move WTP related tests into org.eclipse.wst.xml.core.tests. This provides a cleaner separation and allows for future refactoring to break down into Element, Node, Attribute, and other specific test suites. commit c59762347bc60accceab371bf40738837ae98918 Author: David Carver <dcarver@carver-intalio.(none)> Date: Thu Aug 12 12:34:47 2010 -0700 Fixes and rework for some Attr Implementations. Added DOMException Messages for items not supported by WTP DOM implementation. The way WTP DOM handles attributes is different, it doesn't support the concept of Text Nodes for Attributes.
Updated the WTPTestDocumentBuilderFactory to give the test Document a location so that grammars can be resolved and default values computed (with concurrent changes to ElementImpl). CharacterDataImpl and TextImpl no longer throw INDEX_SIZE_ERR in a couple of places where it's not specified, but should logically throw some sort of boundary exception.
(In reply to comment #20) > I've started work on this again. Current stats are 104 failures, 20 errors > out of 499 tests. > > Work on this is happening in a GitHub clone, and I'll attach a patch once all > tests are passing. If anybody wants to follow along, you can watch the wtp-dom > branch at: > > http://github.com/kingargyle/WTP-Source-Editing/tree/wtp-dom That URL now gives a 404 error. Dave, can you attach a patch for what's done so far? I get the nagging suspicion I just repeated some of your work. 99 failures, 14 errors left in Level 1 suite.
(In reply to comment #26) > (In reply to comment #20) > > I've started work on this again. Current stats are 104 failures, 20 errors > > out of 499 tests. > > > > Work on this is happening in a GitHub clone, and I'll attach a patch once all > > tests are passing. If anybody wants to follow along, you can watch the wtp-dom > > branch at: > > > > http://github.com/kingargyle/WTP-Source-Editing/tree/wtp-dom > > That URL now gives a 404 error. Dave, can you attach a patch for what's done > so far? I get the nagging suspicion I just repeated some of your work. > > 99 failures, 14 errors left in Level 1 suite. Actually, I went looking for it myself, and it looks like I deleted this when I was doing some clean up of my github items. Most of that work would not have applied cleanly anyways due to changes in the code base. So best to start from scratch and implement the changes as you fix them.
