Bug 12560 - W3C XPath test Text_Nodes.svg fails
Summary: W3C XPath test Text_Nodes.svg fails
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks: 12450
  Show dependency treegraph
 
Reported: 2007-02-03 07:49 PST by Alexey Proskuryakov
Modified: 2007-03-11 22:54 PDT (History)
0 users

See Also:


Attachments
proposed fix (3.35 KB, patch)
2007-02-03 08:01 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff
undo the fix (8.29 KB, patch)
2007-03-11 11:34 PDT, Alexey Proskuryakov
adele: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-02-03 07:49:33 PST
dom/svg/level3/xpath/Text_Nodes.svg

The test is supposed to verify that the implementation only returns the first text node in a chain of siblings, but its condition is actually inverted! This is not a conversion artifact, see <http://dev.w3.org/cvsweb/2001/DOM-Test-Suite/tests/level3/xpath/Text_Nodes.xml?rev=HEAD&content-type=text/x-cvsweb-markup> for the original.

Fixing the condition makes the test pass in Opera, but not in Firefox, which apparently doesn't implement this behavior.
Comment 1 Alexey Proskuryakov 2007-02-03 08:01:49 PST
Created attachment 12901 [details]
proposed fix

This behavior looks pretty weird, and is not supported by Firefox, so I have certain doubts about this patch. But at least Opera passes the test.
Comment 2 Darin Adler 2007-02-03 12:08:18 PST
Comment on attachment 12901 [details]
proposed fix

+            if ((node->nodeType() == Node::TEXT_NODE || node->nodeType() == Node::CDATA_SECTION_NODE)) {

isCharacterDataNode() is another way to do the same check -- maybe we should use that.

+        * dom/svg/level3/xpath/Text_Nodes.js: Invert the success condition, as the test appears incorrect.

Should we report this to the W3C?

Please add comments to the test to indicate that we've made local modifications.

r=me
Comment 3 Alexey Proskuryakov 2007-02-03 12:45:34 PST
Committed revision 19389.

(In reply to comment #2)
> isCharacterDataNode() is another way to do the same check -- maybe we should
> use that.

  Hmm... Seems to be less than 100% clear that this includes Text nodes (at least, I had to look that up), so I left the check as is.

> Should we report this to the W3C?

  Sure, will try to report this via mailing list.

> Please add comments to the test to indicate that we've made local
> modifications.

  Done.
Comment 4 Alexey Proskuryakov 2007-02-04 04:04:09 PST
(In reply to comment #3)
>   Sure, will try to report this via mailing list.

Actually, seems like Bugzilla is the right way: <http://www.w3.org/Bugs/Public/show_bug.cgi?id=4297>.
Comment 5 Alexey Proskuryakov 2007-03-11 11:32:15 PDT
I'd like to take this back.

After a more detailed examination, it turns out that we don't match Opera anyway in many cases, and the (draft) spec doesn't fully define the behavior. I cannot figure out what the intended behavior is when the context node is an arbitrary text one. For example, in a fragment such as "<elem>a<![CDATA[b]]>c</elem>", going by "ancestor-or-self" axis from the last text node clearly shouldn't produce [elem, first-text-node]. The situation for "preceding" and "following" axes is similar.

Given the above, and the fact that this behavior isn't implemented in Firefox (including today's Minefield), I suggest that we drop it.
Comment 6 Alexey Proskuryakov 2007-03-11 11:34:12 PDT
Created attachment 13586 [details]
undo the fix
Comment 7 Alexey Proskuryakov 2007-03-11 22:54:09 PDT
Committed revision 20110, marking WONTFIX.