1891792 - Eliminate Inline Event Handlers From FullPageTranslationsPanel

Status: RESOLVED FIXED

(Firefox :: Translations, task)

Description

[Erik Nordin [:nordzilla]]

Description

See Bug 1890547 for more context.
See Bug 1891782 for an example of a similar bug to follow as a guide.

This bug tracks the progress of removing all inline event handlers from the FullPageTranslationsPanel markup.

We need to remove all of the inline event handlers from the FullPageTranslationsPanel markup and instead lazily initialize them in the JS file for the FullPageTranslationsPanel.

---

Steps to implement

• Remove all oncommand, onpopupshown, onpopuphidden inline event handlers from fullPageTranslationsPanel.inc.xhtml
• Lazily initialize all relevant event listeners in fullPageTranslationsPanel.js

---

Tests to implement

• The changes should preserve the exact same behavior as before.
• Preexisting test coverage is sufficient.

Comment 1

[Tom Schuster (MoCo)]

Sorry. I didn't realize that we already had a good-first-bug for this before working on it.

Comment 2

[Tom Schuster (MoCo)]

Attachment: Bug 1891792 - Remove inline oncommand event handlers in FullPageTranslationsPanel. r?nordzilla

Comment 3

[Erik Nordin [:nordzilla]]

(In reply to Tom Schuster (MoCo) [mostly OOO] from comment #1)

Sorry. I didn't realize that we already had a good-first-bug for this before working on it.

Hey Tom,

That's okay! I actually think I marked it as good-first-bug before I realized about the subtle nuances in the way that oncommand is handled differently for <menulist> elements.

You can refer to my review comment regarding the details, but this bug is actually going to be a bit more involved than simply moving the calls from the markup to the JS.

Comment 4

[Tom Schuster (MoCo)]

Gijs just fwiw because I hadn't realized that issue with the command event not triggering on <menulist> items. I will try and see if we overlooked this in another bug before.

Comment 5

[:Gijs (he/him)]

Thanks for the heads up. We don't have a lot of menulists in the browser.xhtml code, so I'm not too worried. There's the content select dropdown (which already uses addEventListener), and the webrtc popup dropdowns for cameras/screens/etc. which we haven't touched and also already use event listeners. It wouldn't hurt to doublecheck the patches we've done already, but I'd be surprised if there was anything there.

Comment 6

[Tom Schuster (MoCo)]

Attachment: Bug 1891792 - Remove inline onclick event handlers in FullPageTranslationsPanel. r?#translations-reviewers

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56bb128841f5
Remove inline oncommand event handlers in FullPageTranslationsPanel. r=nordzilla,translations-reviewers
https://hg.mozilla.org/integration/autoland/rev/aa42311c2c86
Remove inline onclick event handlers in FullPageTranslationsPanel. r=translations-reviewers,nordzilla

Comment 8

[Tom Schuster (MoCo)]

Attachment: Bug 1891792 - Remove inline popup event handlers in FullPageTranslationsPanel. r?#translations-reviewers

Comment 9

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/56bb128841f5
https://hg.mozilla.org/mozilla-central/rev/aa42311c2c86

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93ce570e89e2
Remove inline popup event handlers in FullPageTranslationsPanel. r=translations-reviewers,nordzilla

Comment 11

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/93ce570e89e2