1341338 - feedType is undefined in FeedConverter#handleResult

Status: RESOLVED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[Mark Banner (:standard8)]

In working on no-undef eslint issues, I've discovered there's an issue in FeedConverter#handleResult. `feedType` is undefined, it should be `feed.type` from looking at the code around it.

This was introduced by bug 1109714.

I'm attaching a patch for it, that will also resolve a couple of other no-undef issues as well (these are comments to prompt eslint to know the globals properly).

Comment 1

[Mark Banner (:standard8)]

Attachment: Bug 1341338 - feedType is undefined in FeedConverter#handleResult.

Review commit: https://reviewboard.mozilla.org/r/114146/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/114146/

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Bug 1109714 landed in Firefox 46. Marking status flags accordingly.

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8839526 [details]
Bug 1341338 - feedType is undefined in FeedConverter#handleResult.

https://reviewboard.mozilla.org/r/114146/#review115620

It would be nice if we had a test that hit this FeedConverter codepath. Maybe that could be a good-first-bug?

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a437b95a0de4
feedType is undefined in FeedConverter#handleResult. r=jaws

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a437b95a0de4

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora/Beta approval on this when you get a chance.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 8839526 [details]
Bug 1341338 - feedType is undefined in FeedConverter#handleResult.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1109714
[User impact if declined]: If something other than our default feed viewer was the default action when encountering feeds, and a user loaded something with a feed mimetype, they'd hit it, and it would likely be handled incorrectly.
[Is this code covered by automated tests?]: Not currently, see bug 1341401.
[Has the fix been verified in Nightly?]: Picked up by ESLint no-undef that is soon to be enabled. I don't think this has been verified yet.
[Needs manual test from QE? If yes, steps to reproduce]: See the user impact.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Low risk, small typo fix.
[String changes made/needed]: None

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 8839526 [details]
Bug 1341338 - feedType is undefined in FeedConverter#handleResult.

typo fix found by eslint, aurora53+, beta52+

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/432f86adc83e

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/08169479d28f

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/08169479d28f