1134940 - Remove Reader_foo function names from method declarations in AboutReader.jsm

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect)

Description

[:Margaret Leibovic]

This is an old convention that pre-dates more useful error logging. Let's update our code.

To fix this bug, just remove the function names in AboutReader, e.g. convert this:
handleEvent: function Reader_handleEvent(aEvent) {

To this:
handleEvent: function(aEvent) {

Comment 1

[Lewis Cowper]

I'd quite like to handle this, but if someone wants to take it as their first bug, they're more than welcome.

Comment 2

[:Margaret Leibovic]

(In reply to Lewis Cowper from comment #1)
> I'd quite like to handle this, but if someone wants to take it as their
> first bug, they're more than welcome.

There are always more bugs, so feel free to take this on if you like! It would be a nice clean-up.

Comment 3

[Lewis Cowper]

Attachment: reader_foo-patch.diff

That looks like it should take care of it. :)

Comment 4

[:Margaret Leibovic]

Comment on attachment 8567263 [details] [diff] [review]
reader_foo-patch.diff

Review of attachment 8567263 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! In the future, you should set the review? flag on a patch, instead of using needinfo? on the bug. That will help make sure your patch doesn't get lost in the shuffle! :)

Comment 5

[Lewis Cowper]

Ah, I didn't know whether the review flag was for looking for reviews or responding to them. Thanks for the tip!

Comment 6

[:Margaret Leibovic]

Lewis, your patch failed to apply to the lastest fx-team. Could you update your tree to rebase?

I also just landed a few more AboutReader.jsm changes on fx-team, so unfortunately mozilla-central won't be caught up yet.

Comment 7

[Lewis Cowper]

Sure! I've not got fx-team locally, so I'll take a bit of time to download and set up, it might be tomorrow by the time I get the chance to sort it out.

Comment 8

[Lewis Cowper]

Attachment: fx-team-patch.diff

As it turns out, it was somewhat quicker than I originally thought.

Comment 9

[:Margaret Leibovic]

Comment on attachment 8567350 [details] [diff] [review]
fx-team-patch.diff

Review of attachment 8567350 [details] [diff] [review]:
-----------------------------------------------------------------

Hm, this is still failing to apply. Maybe I was too slow to review it and more changes landed since you posted this :(

Comment 10

[:Margaret Leibovic]

I'm sorry for all the churn! Would you mind rebasing your patch again? I'll pay close attention and make sure to land it for you quickly before it bit-rots again :)

Comment 11

[:Margaret Leibovic]

Comment on attachment 8567350 [details] [diff] [review]
fx-team-patch.diff

Clearing review while waiting for a new patch.

Comment 12

[Molly Howell (she/her) (no longer active)]

Attachment: MozReview Request: Bug 1134940 - Modernize ReaderMode function declarations. r?nalexander

Bug 1134940 - Modernize ReaderMode function declarations. r?nalexander

Comment 13

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8665125 [details]
MozReview Request: Bug 1134940 - Modernize ReaderMode function declarations. r?nalexander

https://reviewboard.mozilla.org/r/20147/#review18087

Looking good.

Comment 14

[Molly Howell (she/her) (no longer active)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=241196881410

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5906d37343fa

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/5906d37343fa

Comment 17

[Molly Howell (she/her) (no longer active)]

Comment on attachment 8665125 [details]
MozReview Request: Bug 1134940 - Modernize ReaderMode function declarations. r?nalexander

Bug 1134940 - Modernize ReaderMode function declarations. r?nalexander

Comment 18

[Molly Howell (she/her) (no longer active)]

Attachment: MozReview Request: Bug: 1208626 Add needed parameter to getHistogramById r?rstrong

Bug: 1208626 Add needed parameter to getHistogramById r?rstrong