Add IETF Atom 1.0 support to Thunderbird

RESOLVED FIXED

Status

MailNews Core
Feed Reader
--
enhancement
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: Robert Sayre, Assigned: Scott MacGregor)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
Atom is finalized, and we have a namespace to use.
(Reporter)

Comment 1

13 years ago
Created attachment 190346 [details] [diff] [review]
Add Atom 1.0 support to Thunderbird
Attachment #190346 - Flags: review?(mscott)
(Reporter)

Comment 2

13 years ago
Created attachment 190348 [details]
OPML file with Atom 1.0  feeds
(Assignee)

Comment 3

13 years ago
Comment on attachment 190346 [details] [diff] [review]
Add Atom 1.0 support to Thunderbird

this worked well for me. Great job as always.

I had one question about:

serializeTextConstruct: function(textElement)

are we doing the right thing with the content variable here? it looks like we
initialize it to null, then we change it to an empty string, then at the end of
the routine we flip it back to null.

Can that be optimized a big more? i.e. start with an empty string and then
return content directly instead of converting it back to null.

I made a couple very minor white space changes including: getting rid of  the
braces around single line if/else clauses.
(Reporter)

Comment 4

13 years ago
(In reply to comment #3) 
> Can that be optimized a big more? i.e. start with an empty string and then
> return content directly instead of converting it back to null.

I've attached something a bit better, but returning null is the correct behavior
if we don't find anything in the content element (the storage code depends on it).

> I made a couple very minor white space changes including: getting rid of  the
> braces around single line if/else clauses.

Always forget that. Fixed.
(Reporter)

Comment 5

13 years ago
Created attachment 190499 [details] [diff] [review]
serializeTextConstruct fixed
Attachment #190346 - Attachment is obsolete: true
(Reporter)

Updated

13 years ago
Attachment #190346 - Flags: review?(mscott)
(Reporter)

Updated

13 years ago
Attachment #190499 - Flags: review?(mscott)
(Assignee)

Comment 6

13 years ago
Comment on attachment 190499 [details] [diff] [review]
serializeTextConstruct fixed

I'm going to change:

+    if (content == "")
+      return null;
+    else
+      return content;

to
// we need to return null and not an empty string
return content ? content : null;

before I check in. Everything else looks good.
Attachment #190499 - Flags: review?(mscott) → review+
(Assignee)

Comment 7

13 years ago
fixed. thanks again for another great patch Robert. Keep 'em coming :)
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird1.1

Updated

9 years ago
Component: RSS → Feed Reader
Product: Thunderbird → MailNews Core
Target Milestone: Thunderbird1.1 → ---
You need to log in before you can comment on or make changes to this bug.