987736 - remove third false param from addEventListener

Status: RESOLVED FIXED

(Webmaker Graveyard :: Popcorn Maker, defect)

Description

[Scott [:thecount] Downe]

In response to bug 897290

We have the third param false used all over the place in our adEventListener calls. This is not needed anymore. You only need a param if it is true.

Comment 1

[AdityaSingh [:MacroMayhem]]

I'd like to work on it. Can you please guide me through the procedure ?

Comment 2

[Jon Buckley [:jbuck]]

Attachment: https://github.com/mozilla/webmaker.org/pull/657/files

Comment 3

[Scott [:thecount] Downe]

Comment on attachment 8397065 [details]
https://github.com/mozilla/webmaker.org/pull/657/files

That is the idea.

However, this ticket is specifically to do it for https://github.com/mozilla/popcorn.webmaker.org

However, it looks to only exist in that one spot on webmaker.org, so I see no reason why not to take this patch for webmaker.org, but keep this ticket open for doing it on popcorn maker too.

Comment 4

[[github robot]]

Commit pushed to master at https://github.com/mozilla/webmaker.org

https://github.com/mozilla/webmaker.org/commit/bc615819f6e7990260f45c8377a832f9f29b55a5
Bug 987736 - Remove third false parameter from addEventListener

Comment 5

[Biraj Karmakar [:biraj]]

Hi Scott,

I am interested also for further thing . Can you guide for solving this bug ?

Comment 6

[Scott [:thecount] Downe]

Hey!

Similar to this patch: https://github.com/mozilla/webmaker.org/pull/657/files

We want to update the uses of addEventListener for https://github.com/mozilla/popcorn.webmaker.org

That is, remove all the third false params. If the param is true, we need to leave it.

Comment 7

[Biraj Karmakar [:biraj]]

Hi Scott 


I have understood but one quick question to you that is there any specific dir or i have to check whole https://github.com/mozilla/popcorn.webmaker.org repo to update the uses of addEventListener . I mean to say that in webmaker the changes made in only one file public/js/pages/details.js

Comment 8

[Matthew Schranz [:mjschranz]]

Anything you should need to change will be in the public/src directory.

Comment 9

[Biraj Karmakar [:biraj]]

@Matthew : thanks for your help.

I have sent the pull request https://github.com/mozilla/popcorn.webmaker.org/pull/537 .

Comment 10

[Scott [:thecount] Downe]

There are some trivial errors here:

Linting public/src/editor/base-editor.js...ERROR
[L227:C60] E024: Unexpected ')'.
        window.addEventListener( "mousedown", onMousedown, );
[L227:C60] E030: Expected an identifier and instead saw ')'.
        window.addEventListener( "mousedown", onMousedown, );
[L227:C61] W116: Expected ')' and instead saw ';'.
        window.addEventListener( "mousedown", onMousedown, );
[L227:C62] W033: Missing semicolon.
        window.addEventListener( "mousedown", onMousedown, ); 

Basically, remove the final comma on those four lines should clean it up.

Comment 11

[Scott [:thecount] Downe]

Err, that's all on one line. So just remove the comma on that one line.

Comment 12

[Scott [:thecount] Downe]

Ah, I missed this in the initial description of the bug.

It's also safe to remove the false param from removeEventListener

Comment 13

[Scott [:thecount] Downe]

Finally, looks like you'll need to rebase as the commit you initially based this on, being 9c6fd75254444b4c174719e5add4b3312b8e4195 or https://github.com/mozilla/popcorn.webmaker.org/commit/9c6fd75254444b4c174719e5add4b3312b8e4195 was busted and got reverted.

I tested it, and doing a rebase onto your patch merges cleanly.

Let me know if you have questions about rebasing and merges! Happy to help.

Comment 14

[akshaytiwari.003]

I've removed the false parameter from removeEventListener. Below is the link and i've attached the link as attachment too.
https://github.com/mozilla/webmaker.org/pull/929

Comment 15

[akshaytiwari.003]

Attachment: Regarding the bug 985536

Comment 16

[akshaytiwari.003]

(In reply to akshaytiwari.003 from comment #15)
> Created attachment 8468583 [details] [review]
> Regarding the bug 987736 * sorry

Comment 17

[Scott [:thecount] Downe]

Comment on attachment 8468583 [details] [review]
Regarding the bug 985536

This looks fine to me.

Landing this.

Comment 18

[Scott [:thecount] Downe]

I also noticed https://github.com/mozilla/popcorn.webmaker.org/pull/537/files got closed and never landed. I'll see if I can recover it.

Then we can probably close this ticket.

Comment 19

[[github robot]]

Commit pushed to master at https://github.com/mozilla/popcorn.webmaker.org

https://github.com/mozilla/popcorn.webmaker.org/commit/90d147f19f8ffb2191779e162db3cd1d4c1160d8
Bug 987736 - Remove third false parameter from addEventListener

Comment 20

[Scott [:thecount] Downe]

Closing this now, bunch of stuff landed :D

Comment 21

[akshaytiwari.003]

Thank you.