Closed Bug 1504725 Opened 2 years ago Closed 2 years ago

Error when trying to restart into new language on Beta


(Firefox :: Preferences, defect, P1)




Firefox 65
Tracking Status
firefox64 --- fixed
firefox65 --- fixed


(Reporter: mstriemer, Assigned: mstriemer)


(Blocks 1 open bug)



(1 file)

Nothing happens when clicking the "Apply and restart" button after switching browser languages on Beta.

There is an error in the console: ReferenceError: event is not defined
Smaug, do you know why this worked on Nightly but doesn't work on Beta? It seems concerning that the `event` object is available in Nightly but not on Beta.

I tried to do a mozregression with jaws's help it doesn't appear to work with mozilla-beta right now.

The relevant bits seem to be:

  <!-- in-content/main.xul [1] -->
  <button class="message-bar-button" oncommand="gMainPane.confirmBrowserLanguageChange()"/>

  // in-content/main.js [2]
  var gMainPane = { // ...
    confirmBrowserLanguageChange() {
      // `event` is undefined here.
      let localesString = ("locales") || "").trim();
      // ...

Flags: needinfo?(bugs)
Pushed by
Explicitly pass event to language change handler r=jaws
Comment on attachment 9022645 [details]
Bug 1504725 - Explicitly pass event to language change handler r?jaws

No QE needed since this is behind a pref and there are other bugs that will be verified (currently waiting to test those blocked on this one).

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: None

User impact if declined: The restart into new language feature won't work.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Makes implicit argument passed explicitly.

String changes made/needed: None
Attachment #9022645 - Flags: approval-mozilla-beta?
Yeah, window.event is old IEism and shouldn't be used. We're adding support for it just for web compatibility. Note, window.event isn't available for example when dealing with Shadow DOM.
Flags: needinfo?(bugs)
Just got the mozregression working, tracked it down to bug 1493869. So having this work on Nightly but not Beta is expected but sounds like this bug could easily happen again.

Is there something we should do to catch these bugs before they hit Beta? Should we add a lint rule for this?
Flags: needinfo?(bugs)
If the issue could be caught by lint, then yes. It is in general bad practice to rely on
"global variable" -like window.event. And as I said, window.event won't work when dealing with shadow DOM.
Flags: needinfo?(bugs)
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
bug 1505031 is another case of beta-only breakage due to this footgun.  That's pretty scary...
Comment on attachment 9022645 [details]
Bug 1504725 - Explicitly pass event to language change handler r?jaws

trivial fix, approved for 64.0b8
Attachment #9022645 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1504890
There's a lint rule from bug 1504890 now, so hopefully it doesn't happen again.
You need to log in before you can comment on or make changes to this bug.