1228282 - In some configurations feed reader breaks completely

Status: RESOLVED FIXED

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

Description

[:Gijs (he/him)]

STR:

1. clean profile
2. set the following prefs:

browser.feeds.handler.default = "client"
browser.feeds.handlers.application = "path/to/some/app/like/Thunderbird.app(or.exe)"

3. open http://feeds.bbci.co.uk/news/rss.xml?edition=uk

ER:

see feed

AR:

Everything is broken. Errors in console:

ReferenceError: aMenuitem is not defined FeedWriter.js:639:5
Use of Mutation Events is deprecated. Use MutationObserver instead. menulist.xml:240:16
ReferenceError: aMenuitem is not defined FeedWriter.js:639:5
NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "aMenuitem is not defined" {file: "jar:file:///Applications/Firefox.app/Contents/Resources/browser/omni.ja!/components/FeedWriter.js" line: 639}]'[JavaScript Error: "aMenuitem is not defined" {file: "jar:file:///Applications/Firefox.app/Contents/Resources/browser/omni.ja!/components/FeedWriter.js" line: 639}]' when calling method: [nsIDOMGlobalPropertyInitializer::init] subscribe.js:13:0
TypeError: this._feedWriter is null

Comment 1

[:Gijs (he/him)]

Spot what's wrong here:

https://hg.mozilla.org/mozilla-central/file/35916735b8afc5b0732e00f9aeb56bf846bba7f4/browser/components/feeds/FeedWriter.js#l646

   646   _initMenuItemWithFile: function(aMenuItem, aFile) {
   647     var label = this._getFileDisplayName(aFile);
   648     var image = this._getFileIconURL(aFile);
   649     aMenuitem.setAttribute('label', label);
   650     aMenuitem.setAttribute('image', image);
   651   },

I can't wait for us to have proper linting in place so we catch this kind of cruft.

Comment 2

[:Gijs (he/him)]

Attachment: MozReview Request: Bug 1228282 - fix use of aMenuItem in FeedWriter's _initMenuItemWithFile to unbreak feed reader UI, r?mak

Bug 1228282 - fix use of aMenuItem in FeedWriter's _initMenuItemWithFile to unbreak feed reader UI, r?mak

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8692467 [details]
MozReview Request: Bug 1228282 - fix use of aMenuItem in FeedWriter's _initMenuItemWithFile to unbreak feed reader UI, r?mak

https://reviewboard.mozilla.org/r/26267/#review23701

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5fe77514a40b

Comment 5

[:Gijs (he/him)]

Comment on attachment 8692467 [details]
MozReview Request: Bug 1228282 - fix use of aMenuItem in FeedWriter's _initMenuItemWithFile to unbreak feed reader UI, r?mak

Approval Request Comment
[Feature/regressing bug #]: bug 1018062
[User impact if declined]: feed reader completely broken in some circumstances
[Describe test coverage new/current, TreeHerder]: well, clearly the test coverage isn't good enough... but rather than testing for the specific thing here (which is likely "tricky" to get right on infra because it relies on executables existing to even hit this path) I would prefer to invest in linting. I'll post to fx-dev about it.
[Risks and why]: zero risk patch. No, seriously. Look at the patch.
[String/UUID change made/needed]: no.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5fe77514a40b

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8692467 [details]
MozReview Request: Bug 1228282 - fix use of aMenuItem in FeedWriter's _initMenuItemWithFile to unbreak feed reader UI, r?mak

Typo fIx. Please uplift thIs sImple patch to aurora and beta.

Comment 8

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/4b9e76d10c53

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/e98a7ddc5fdc

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/e98a7ddc5fdc