Closed
Bug 998063
Opened 10 years ago
Closed 10 years ago
Feed parser should support media:thumbnail
Categories
(Toolkit :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: wesj, Assigned: wesj)
References
Details
Attachments
(1 file, 1 obsolete file)
7.28 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
The new Firefox Hub addons for Android are using the Feed Parser for RSS feeds. None of the feeds I've seen show thumbnails, but a little digging showed (some) feeds using the media:thumbnail markup. We should support it!
Assignee | ||
Comment 2•10 years ago
|
||
Didn't realize the tests here basically just made sure we could parse some files. Added a media:thumbnail bit to them. Removed some sneaky logging as well :)
Attachment #8408589 -
Attachment is obsolete: true
Attachment #8408589 -
Flags: review?(mak77)
Attachment #8408630 -
Flags: review?(mak77)
Comment 3•10 years ago
|
||
Comment on attachment 8408630 [details] [diff] [review] Patch Review of attachment 8408630 [details] [diff] [review]: ----------------------------------------------------------------- this code hurts my eyes every time :) ::: toolkit/components/feeds/FeedProcessor.js @@ +564,5 @@ > if (bagHasKey(contentElement, "type")) { > enc.setPropertyAsAString("type", contentElement.getPropertyAsAString("type")); > + } else if (mediaType == "mediathumbnail") { > + // thumbnails won't have a type, but default to image types > + enc.setPropertyAsAString("type", "image/*"); How do we distinguish this thumbnail from a media:content having type="image/*"? Maybe there should be an additional "thumbnail" boolean property in case the consumer wants to distinguish them? ::: toolkit/components/feeds/test/xml/rss2/mrss_content.xml @@ +9,5 @@ > <channel> > <item> > > +<media:content fileSize="24986239" type="video/mpeg" url="http://dallas.example.com/joebob_050689.mpeg" /> > +<media:thumbnail url="http://dallas.example.com/joebob_050689.mpeg" width="75" height="50" time="12:05:01.123"/> from what I see, thumbnail should always be an image. Doesn't make a difference here, but for coherence should probably be .jpg Also, there's 2 spaces before width.
Attachment #8408630 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/253acf385229
Comment 5•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/253acf385229
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•