Closed
Bug 118865
Opened 23 years ago
Closed 23 years ago
Asserting an rdf int literal writes malformed xml
Categories
(Core Graveyard :: RDF, defect, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9.8
People
(Reporter: bugzilla, Assigned: waterson)
References
Details
Attachments
(2 files, 4 obsolete files)
10.89 KB,
patch
|
waterson
:
review+
|
Details | Diff | Splinter Review |
14.35 KB,
patch
|
Details | Diff | Splinter Review |
I've got code that looks like this: nsCOMPtr<nsIRDFNode> percent; gRDFService->GetResource(aTargetPath, getter_AddRefs(res)); nsresult rv = gRDFService->GetIntLiteral(someInteger, getter_AddRefs (percent)); if (NS_FAILED(rv)) return rv; rv = Assert(res, gNC_ProgressPercent, percent, PR_TRUE); The output in the datasource file is of the form <NC:ProgressPercent </RDF:Description> That's it. The correct output, of course, is <NC:ProgressPercent>someInteger</NC:ProgressPercent>. Let's see if I can get an attachable testcase together.
Assignee | ||
Comment 1•23 years ago
|
||
Eww. N.B. that we'll need to add meta-information (like `moz:parseType') for this to be parsed back as an integer.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.8
Comment 2•23 years ago
|
||
I don't see why we should. Why not just have SerializeChildAssertion() QI to different node types and print accordingly? i.e. expand on http://lxr.mozilla.org/seamonkey/source/rdf/base/src/nsRDFXMLSerializer.cpp#388 (in fact there's an NS_ERROR just below that to this effect.) Unless something trickier is at work here.
Comment 3•23 years ago
|
||
>I don't see why we should.
s/should/should have to/
Assignee | ||
Comment 4•23 years ago
|
||
The reason we need a `parseType' attribute is so that we will reconstitute an nsIRDFInt object when we read this back in. (Otherwise, we'll just parse it back as a string.) We might as well do dates while we're at it, too.
Comment 5•23 years ago
|
||
D'oh, you're right. I should learn to read comments :\ eww. Time to go hunt around and see if there's any precedent for simple typing in RDF. I have a feeling I may have deleted a lot of www-rdf-interest messages on the subject at one point.
Assignee | ||
Comment 6•23 years ago
|
||
This patch adds support for parsing date and integer literals, as well as `RDF:parseType="resource"'. Yeah, I'm evil: I'm using the `RDF:parseType' attribute to get this done. But I figure that's what it's there for. I need to update to pick up harishd's changes: may need to post another patch if there are merge conflicts.
Assignee | ||
Comment 7•23 years ago
|
||
Sho'nuff, had some conflicts after harishd's landing. This patch merges those, removes some unused code, and capitalizes the parseType attribute values (`Resource', `Integer', `Date'), paying lip-service to the spec.
Attachment #64251 -
Attachment is obsolete: true
Assignee | ||
Comment 8•23 years ago
|
||
One more try. I figure we might as well not emit invalid RDF if someone later adds a new node type (like nsIRDFBlob). Fixed up <RDF:li> emits to include `parseType' when appropriate. Emit `parseType' instead of `RDF:parseType'.
Attachment #64256 -
Attachment is obsolete: true
Comment 9•23 years ago
|
||
We probably shouldn't be using a naked "parseType" or a "parseType" in the RDF namespace, since it would be spec violation to use values for it other than "Literal" or "Resource" (and might horribly confuse other parsers if they ever tried to read our emitted RDF).
Assignee | ||
Comment 10•23 years ago
|
||
damn you, tingley! ;-) You are, of course, right. I was being lazy. I'll go the extra mile and move Integer and Date to the `http://home.netscape.com/NC-rdf#' namespace.
Assignee | ||
Comment 11•23 years ago
|
||
Rather than overloading the naked `parseType' attribute, we'll use `nc:parseType' instead. It costs a bit more code, but it's future-proof.
Attachment #64257 -
Attachment is obsolete: true
Comment 12•23 years ago
|
||
> // XXX it doesn't support nsIRDFResource _or_ nsIRDFLiteral??? > // We should serialize nsIRDFInt, nsIRDFDate, etc... Perhaps delete the above comments? > static const char *gMonths[] = { > static const char *gDays[] = { > rdf_FormatDate Isn't there something in either NSPR (prtime.c and family) or I18N that could help with this? Its kind of a bummer to have hardcoded names for months and days, since they can't be internationalized/localized.
Assignee | ||
Comment 13•23 years ago
|
||
I'll fix the comments. Thanks for pointing that out. I could use PR_FormatTime[USEnglish]; I didn't because of the NO_NSPR_10_SUPPORT define -- I guess it really doesn't matter. I'll change it. I couldn't find any routines to _parse_ dates in I18n, otherwise I'd have used those. But, FWIW, I'm not worried about localizing these dates: this is the format that we use to _save_ data after it's been read once -- note that dates are always saved as GMT, which isn't exactly user-friendly. (In fact, I was tempted to output them as seconds from the Unix epoch, but didn't just so debugging would be easier down the road.)
Comment 14•23 years ago
|
||
Comment on attachment 64309 [details] [diff] [review] address tingley's comments Chris: WORKSFORME r=rjc BTW
Attachment #64309 -
Flags: review+
Assignee | ||
Comment 15•23 years ago
|
||
Use PR_FormatTimeUSEnglish instead of hand-rolled cruft.
Attachment #64309 -
Attachment is obsolete: true
Assignee | ||
Comment 16•23 years ago
|
||
Comment on attachment 64400 [details] [diff] [review] ...and rjc's comments I assume this is still r=rjc
Attachment #64400 -
Flags: review+
Comment 17•23 years ago
|
||
Comment on attachment 64400 [details] [diff] [review] ...and rjc's comments r=rjc
Comment 18•23 years ago
|
||
sr=hyatt
Assignee | ||
Comment 19•23 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 20•23 years ago
|
||
I hate to be a bastard about process, but the portions of this checkin that fixed bug 33197 were never posted here as a patch and AFAIK not reviewed. Did rjc get a chance to look at them before they went in? I'm probably totally off-base, but I'm slightly confused. Sorry for the bother in advance :)
Assignee | ||
Comment 21•23 years ago
|
||
Yeah, you're right. :-( I was maintaining two sets of diffs (to different sets of files) in the same tree, and didn't include the changes to nsRDFContentSink.cpp in either set. (And that was the whole point of posting the ``merge with harishd's changes'' patch in attachment 64256 [details] [diff] [review]!) Here they are, for the record. If you have any post hoc suggestions, we can discuss them.
Assignee | ||
Comment 22•23 years ago
|
||
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•