nsRDFXMLSerializer should emit typed nodes

RESOLVED FIXED in mozilla0.9.5

Status

()

Core
RDF
P4
normal
RESOLVED FIXED
16 years ago
14 years ago

People

(Reporter: Chase Tingley, Assigned: Chris Waterson)

Tracking

Trunk
mozilla0.9.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

16 years ago
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.
(Reporter)

Comment 1

16 years ago
Created attachment 47802 [details] [diff] [review]
patch
(Reporter)

Comment 2

16 years ago
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).
(Assignee)

Comment 3

16 years ago
Looks good, r/sr=waterson. cc'ing some other reviewers who are familiar with RDF.
Status: NEW → ASSIGNED
Keywords: patch
Priority: -- → P4
Target Milestone: --- → mozilla0.9.5
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.
(Reporter)

Comment 5

16 years ago
Created attachment 50119 [details] [diff] [review]
patch - cleaned up shaver's nits
(Reporter)

Comment 6

16 years ago
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.
(Reporter)

Comment 7

16 years ago
Created attachment 50128 [details] [diff] [review]
patch - cleaned up for real (note change to rv handling)
(Reporter)

Comment 8

16 years ago
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 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
Attachment #50128 - Flags: review+
(Assignee)

Comment 10

16 years ago
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...
(Assignee)

Comment 11

16 years ago
Created attachment 50140 [details] [diff] [review]
waterson's nits
(Reporter)

Comment 12

16 years ago
That looks good.  Thanks, waterson.
(Assignee)

Comment 13

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

ok, we'll call this ``submitted by tingley'', sr=waterson
Attachment #50140 - Flags: superreview+
(Assignee)

Comment 14

16 years ago
shaver, could you re-review attachment 50140 [details] [diff] [review]?
(Assignee)

Comment 15

16 years ago
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 on attachment 50140 [details] [diff] [review]
waterson's nits

r=shaver.
Attachment #50140 - Flags: review+
(Assignee)

Comment 17

16 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 18

14 years ago
tever is not RDF QA anymore
QA Contact: tever → nobody
You need to log in before you can comment on or make changes to this bug.