Last Comment Bug 466378 - rdf:nodeID not working when node appears as object before as subject in RDF/XML source
: rdf:nodeID not working when node appears as object before as subject in RDF/X...
Status: RESOLVED FIXED
: fixed1.9.1
Product: Core
Classification: Components
Component: RDF (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.9.1b3
Assigned To: Florian Hatat
:
:
Mentors:
Depends on:
Blocks: 11650
  Show dependency treegraph
 
Reported: 2008-11-23 09:46 PST by Florian Hatat
Modified: 2008-12-11 03:31 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix (713 bytes, patch)
2008-11-23 09:52 PST, Florian Hatat
axel: review-
Details | Diff | Splinter Review
Same patch, but check rv before adding the resource to the map (762 bytes, patch)
2008-11-23 10:07 PST, Florian Hatat
axel: review+
shaver: superreview+
mbeltzner: approval1.9.1+
Details | Diff | Splinter Review

Description Florian Hatat 2008-11-23 09:46:33 PST
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.0.3) Gecko/2008092816 Iceweasel/3.0.3 (Debian-3.0.3-3)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; fr; rv:1.9.0.4) Gecko/2008102920 Firefox/3.0.4

The RDF parser generates anonymous resources when it sees "rdf:nodeID".

This works properly when a <rdf:Description rdf:nodeID="foo"> appears in the file before the id "foo" is referenced. Yet, when the rdf:nodeID appears as the object of a predicate before being described in a <rdf:Description>, the parser generates two different resources for the same identifier.

Such an input is valid according to the RDF spec, see the section about nodeID: http://www.w3.org/TR/rdf-syntax-grammar/#section-Syntax-blank-nodes

I think this should block bug 11650.

Reproducible: Always

Steps to Reproduce:
1. Load the following data (example from the RDF spec):

<?xml version="1.0"?>
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:dc="http://purl.org/dc/elements/1.1/"
         xmlns:ex="http://example.org/stuff/1.0/">
  <rdf:Description rdf:about="http://www.w3.org/TR/rdf-syntax-grammar"
                   dc:title="RDF/XML Syntax Specification (Revised)">
    <ex:editor rdf:nodeID="abc"/>
  </rdf:Description>

  <rdf:Description rdf:nodeID="abc"
                   ex:fullName="Dave Beckett">
    <ex:homePage rdf:resource="http://purl.org/net/dajobe/"/>
  </rdf:Description>
</rdf:RDF>


2. Serialize it with "ds.FlushTo(...)".
Actual Results:  
For the moment, FlushTo writes the following file. Note the different resources "rdf:#$Y+Ujz3" and "rdf:#$X+Ujz3".

<?xml version="1.0"?>
<RDF:RDF xmlns:dc="http://purl.org/dc/elements/1.1/"
         xmlns:ex="http://example.org/stuff/1.0/"
         xmlns:NC="http://home.netscape.com/NC-rdf#"
         xmlns:RDF="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
  <RDF:Description RDF:about="rdf:#$Y+Ujz3"
                   ex:fullName="Dave Beckett">
    <ex:homePage RDF:resource="http://purl.org/net/dajobe/"/>
  </RDF:Description>
  <RDF:Description RDF:about="http://www.w3.org/TR/rdf-syntax-grammar"
                   dc:title="RDF/XML Syntax Specification (Revised)">
    <ex:editor RDF:resource="rdf:#$X+Ujz3"/>
  </RDF:Description>
</RDF:RDF>

Expected Results:  
We should get instead something like that:

  <RDF:Description RDF:about="rdf:#$X+Ujz3"
                   ex:fullName="Dave Beckett">
    <ex:homePage RDF:resource="http://purl.org/net/dajobe/"/>
  </RDF:Description>
  <RDF:Description RDF:about="http://www.w3.org/TR/rdf-syntax-grammar"
                   dc:title="RDF/XML Syntax Specification (Revised)">
    <ex:editor RDF:resource="rdf:#$X+Ujz3"/>
  </RDF:Description>

Note that the previous example works fine if you swap the two <RDF:Description>.
Comment 1 Florian Hatat 2008-11-23 09:52:53 PST
Created attachment 349648 [details] [diff] [review]
Proposed fix

I found bug 232623 which introduced the rdf:nodeID feature. I'm not familiar at all with Mozilla's code, yet the following lines of its patch look suspect to my eyes:

+      if (!*aResource) {
+          mNodeIDMap.Put(nodeID,*aResource);
+          return gRDFService->GetAnonymousResource(aResource);

It seems that we're associating "nodeID" with a null resource in "mNodeIDMap", then we create the anonymous resource. Which, I think, will make the parser create another resource the next time it sees the same "nodeID".
Comment 2 Axel Hecht 2008-11-23 10:01:42 PST
Comment on attachment 349648 [details] [diff] [review]
Proposed fix

Nice catch.

r-, though, we shouldn't put *aResource into mNodeIDMap on failure. Put in a 

if (NS_FAILED(rv)) return rv;

and then no explicit return in the if clause?
Comment 3 Florian Hatat 2008-11-23 10:07:47 PST
Created attachment 349649 [details] [diff] [review]
Same patch, but check rv before adding the resource to the map
Comment 4 Axel Hecht 2008-11-23 10:23:27 PST
Comment on attachment 349649 [details] [diff] [review]
Same patch, but check rv before adding the resource to the map

r=me, this needs sr still. Trying shaver. Mike, I think this is small and low-risk enough to take in RDF-land.
Comment 5 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-11-23 11:08:05 PST
Comment on attachment 349649 [details] [diff] [review]
Same patch, but check rv before adding the resource to the map

sr=shaver, just for nostalgia's sake
Comment 6 Mike Beltzner [:beltzner, not reading bugmail] 2008-11-28 12:11:26 PST
Comment on attachment 349649 [details] [diff] [review]
Same patch, but check rv before adding the resource to the map

a191=beltzner
Comment 7 Axel Hecht 2008-12-08 03:45:30 PST
Landed on mozilla-central for baking, http://hg.mozilla.org/mozilla-central/rev/f4ca2f598902. Leaving open to remind me on 1.9.1
Comment 8 Axel Hecht 2008-12-11 03:31:43 PST
Landed on 1.9.1, too, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/0fcb3bd9bd58.

Thanks for the patch, Florian.

FIXED and fixed1.9.1.

Note You need to log in before you can comment on or make changes to this bug.