Closed Bug 1345141 Opened 8 years ago Closed 8 years ago

Miscellaneous feed code deduplication, modernization, simplification, and fix of a deprecation

Categories

(MailNews Core :: Feed Reader, enhancement)

Product:
MailNews Core
Component:
Feed Reader
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 55.0

People

(Reporter: alta88, Assigned: alta88)

Details

Attachments

(4 files, 2 obsolete files)

feedFactory.patch
4.33 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
parser.patch
21.76 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
newFeed.patch
32.04 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
xmlbase.patch
7.43 KB, patch
alta88
: review+
Details | Diff | Splinter Review
No description provided.
Attached patch newFeed.patch (obsolete) — — Splinter Review
Assignee: nobody → alta88
Attachment #8844507 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8844507 [details] [diff] [review] newFeed.patch Part1: Deduplicate and simplify code for new Feed object creation
Bug 1345141 - Part2: Modernize feed xpcom factory code
Attachment #8844508 - Flags: review?(mkmelin+mozilla)
Attached patch parser.patch — — Splinter Review
Bug Bug 1345141 - Part3: Ensure parser returns array, use includes not indexof, use for..of in node loops
Attachment #8844511 - Flags: review?(mkmelin+mozilla)
Attached patch xmlbase.patch (obsolete) — — Splinter Review
Bug 1345141 - Part4: - Remove baseURI from Atom parser, as xml:base is deprecated by Bug 903372; handle relative urls
Attachment #8844515 - Flags: review?(mkmelin+mozilla)
Attached patch newFeed.patch — — Splinter Review
Bug 1345141 - Part1: Deduplicate and simplify code for new Feed object creation
Attachment #8844507 - Attachment is obsolete: true
Attachment #8844507 - Flags: review?(mkmelin+mozilla)
Attachment #8845011 - Flags: review?(mkmelin+mozilla)
Attachment #8844508 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8844511 - Flags: review?(mkmelin+mozilla) → review+
Comment on attachment 8844515 [details] [diff] [review] xmlbase.patch Review of attachment 8844515 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/extensions/newsblog/content/feed-parser.js @@ +580,4 @@ > tags = this.childrenByTagNameNS(channel, FeedUtils.ATOM_IETF_NS, "link"); > + aFeed.link = this.findAtomLink("self", tags, contentBase); > + if (!aFeed.link) > + aFeed.link = this.findAtomLink("alternate", tags, contentBase); would be slightly nicer as: aFeed.link = this.findAtomLink("self", tags, contentBase) || this.findAtomLink("alternate", tags, contentBase); @@ +695,3 @@ > } > else > + item.xmlContentBase = contentBase; while you're here, please ensure all these if-elses have braces
Attachment #8844515 - Flags: review?(mkmelin+mozilla) → review+
Attachment #8845011 - Flags: review?(mkmelin+mozilla) → review+
Attached patch xmlbase.patch — — Splinter Review
updated for comments.
Attachment #8844515 - Attachment is obsolete: true
Attachment #8846657 - Flags: review+
xmlbase.patch should be applied last.
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: