Closed Bug 1493869 Opened 3 years ago Closed 3 years ago
.event behind a pref and disable by default for release versions
Heard from the webcompat team that bug 1479964 has caused problems after we implemented window.event in bug 218415. I believe any fix to bug 1479964 is probably too late to uplift, so I think it would probably be better just disable the new API by default on releases and continue figuring out the way around on Nightly.
Karl, are you ok doing this from webcompat point of view?
Comment on attachment 9011682 [details] Bug 1493869 - Put window.event behind a pref and disable it by default for release versions. r=smaug Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.) has approved the revision.
Attachment #9011682 - Flags: review+
(In reply to Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.) from comment #2) > Karl, are you ok doing this from webcompat point of view? Speaking for Karl, yes. Shipping window.event without event.keyCode in keypress is too risky. Sadly we learned this pretty late...
(we'll need to document that this is not shipping in 63, but will be gated in Nightly (I guess, given this patch) until we can ship Bug 1479964)
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/835be4f82638 Put window.event behind a pref and disable it by default for release versions. r=smaug
Comment on attachment 9011682 [details] Bug 1493869 - Put window.event behind a pref and disable it by default for release versions. r=smaug Approval Request Comment [Feature/Bug causing the regression]: bug 218415 [User impact if declined]: have usability issue on some websites due to bug 1479964 [Is this code covered by automated tests?]: some web-platform tests check the added feature, but nothing really checks it being disabled [Has the fix been verified in Nightly?]: no, just landed, and non-relevant, because the feature in question will stay in Nightly, only beta (and release) will have it disabled. [Needs manual test from QE? If yes, steps to reproduce]: see bug 1479964 [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: it just puts the API newly added in firefox 63 behind a pref [String changes made/needed]: n/a
Attachment #9011682 - Flags: approval-mozilla-beta?
Comment on attachment 9011682 [details] Bug 1493869 - Put window.event behind a pref and disable it by default for release versions. r=smaug Keep dom.window.event.enabled to false on beta/release as the window.event recent change caused multiple webcompat bugs for our upcoming release, uplift approved for 63 beta 10, thanks!
Attachment #9011682 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated the site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2018/support-for-event-returnvalue-has-been-added/
Note to the docs team: I've added a note at the bottom of https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#APIs (see the removals section) to cover this. When you come to document this, it basically just needs a compat data update and explanation in the page.
I've now documented this: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/63#APIs https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/64#APIs I ended up adding to the Fx63 release note to say that it had been added but then quickly hidden behind the flag in non-Nightly, but I also kept the note in 64to say it had been removed, but added that it was actually added in the Fx63 cycle. I did this because it was added late in the 63 release cycle, so people might have missed it as it was added late into the 63 notes. I also made the change in the browser compat data https://github.com/mdn/browser-compat-data/pull/3097 Let me know if that is all OK. Thanks!
You need to log in before you can comment on or make changes to this bug.