Closed Bug 1493869 Opened 6 years ago Closed 6 years ago

Put window.event behind a pref and disable by default for release versions

Categories

(Core :: DOM: Events, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox63 + fixed
firefox64 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

(Keywords: dev-doc-complete, site-compat, Whiteboard: [webcompat:p1])

Attachments

(1 file)

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?
Flags: needinfo?(kdubost)
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+
Tracking for 63
Keywords: site-compat
(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...
Flags: needinfo?(kdubost)
(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)
Keywords: dev-doc-needed
Priority: -- → P2
Flags: webcompat+
Whiteboard: [webcompat:p1]
Pushed by xquan@mozilla.com:
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?
https://hg.mozilla.org/mozilla-central/rev/835be4f82638
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
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+
See Also: → 1493098
See Also: → 1494957
See Also: → 1504890
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.

Attachment

General

Created:
Updated:
Size: