97775 - nsRDFXMLSerializer should emit typed nodes

Status: RESOLVED FIXED

(Core Graveyard :: RDF, defect, P4)

Description

[Chase Tingley]

When we have a valid namespace for 'foo', emit the shorthand 
<foo:bar about="http://some/">
  <prop>14</prop>
</foo:bar>

rather than
<RDF:Description about="http://some/">
  <RDF:type resource="http://path/to/foo/bar" />
  <prop>14</prop>
</RDF:Description>

I noticed this while looking into bug 15006.

Comment 1

[Chase Tingley]

Attachment: patch

Comment 2

[Chase Tingley]

If we can't get a namespace prefix, just treat the node as a normal description
with properties (one of which just happens to be rdf:type).

Comment 3

[Chris Waterson]

Looks good, r/sr=waterson. cc'ing some other reviewers who are familiar with RDF.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 47802 [details] [diff] [review]
patch

>+    rv = mDataSource->GetTarget(aResource, kRDF_type, 
>+                PR_TRUE, getter_AddRefs(target));

(Indentation glitch.)

>+    if (rv == NS_OK)
>+        isTypedNode = PR_TRUE;
>+    else if (rv != NS_RDF_NO_VALUE)
>+        return rv;

Are you sure you don't want to check NS_SUCCEEDED instead 
of comparing against NS_OK?

Tidy the indentation and either use NS_SUCCEEDED or school me
on why not, and sr=shaver.

Comment 5

[Chase Tingley]

Attachment: patch - cleaned up shaver's nits

Comment 6

[Chase Tingley]

Argh, I think I missed an indent.  I'll do this once more.

Incidentally, I have no cvs access.  Waterson, can you check this in for me when
it's ready?  Thanks.

Comment 7

[Chase Tingley]

Attachment: patch - cleaned up for real (note change to rv handling)

Comment 8

[Chase Tingley]

The return codes are more interesting than I realized -- NS_RDF_NO_VALUE is
actually a success code.  This actually makes the code slightly tighter. 

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 50128 [details] [diff] [review]
patch - cleaned up for real (note change to rv handling)

I like that a lot.  Thanks, tingley.  r=shaver

Comment 10

[Chris Waterson]

I like the patch! A couple of nits:

- I tweaked your local variable names a bit, using ``typeNode'' and ``type''
  instead of ``target'' and ``targetType''. Hope that's okay.

- I don't think we ought to hard-fail if you can't QI() the |typeNode| to
  a resource. Some goofball may have done something like this:

  <RDF:Description RDF:type="foop" />

  This would create a literal ``foop'' on the element, and we'd blow up
  serializing it.

- We ought to just check for a target at the end of the rdf:type arc, rather
  than asking for a specific return value. The eases the onus on datasource
  implementers (e.g., it'd be harder -- although not impossible -- to return
  success codes from a datasource implemented in JS).

- gcc pointed out an unused variable, which I removed.

Attaching a patch to fix the above...

Comment 11

[Chris Waterson]

Attachment: waterson's nits

Comment 12

[Chase Tingley]

That looks good.  Thanks, waterson.

Comment 13

[Chris Waterson]

Comment on attachment 50140 [details] [diff] [review]
waterson's nits

ok, we'll call this ``submitted by tingley'', sr=waterson

Comment 14

[Chris Waterson]

shaver, could you re-review attachment 50140 [details] [diff] [review]?

Comment 15

[Chris Waterson]

FWIW, I've tested the above with the following RDF, and it round-trips each
correctly:

Case 1: untyped node

<?xml version="1.0"?>
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:nc="http://home.netscape.com/NC-rdf#">
  <rdf:Description ID="foo" nc:foo="bar" />
</rdf:RDF>


Case 2: typed node

<?xml version="1.0"?>
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:nc="http://home.netscape.com/NC-rdf#">
  <nc:root ID="foo" />
</rdf:RDF>


Case 3: rdf:type as explicit resource (output as case 2, above)

<?xml version="1.0"?>
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:nc="http://home.netscape.com/NC-rdf#">
  <rdf:Description ID="foo">
    <rdf:type resource="http://home.netscape.com/NC-rdf#root" />
  </rdf:Description>
</rdf:RDF>


Case 4: rdf:type as a literal

<?xml version="1.0"?>
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
         xmlns:nc="http://home.netscape.com/NC-rdf#">
  <rdf:Description ID="foo">
    <rdf:type>http://home.netscape.com/NC-rdf#root</rdf:type>
  </rdf:Description>
</rdf:RDF>

Comment 16

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 50140 [details] [diff] [review]
waterson's nits

r=shaver.

Comment 17

[Chris Waterson]

Fix checked in.

Comment 18

[benc]

tever is not RDF QA anymore