1607686 - Add "beforeinput" to "Event Listener Breakpoints"

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement, P3)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'll implement beforeinput event at bug 970802. That's sometimes a pair with input event, but not always. Currently, input event is under "Keyboard" group. So, perhaps, beforeinput should also be added into the group.

FYI: beforeinput event will be landed as disabled by default even in Nightly channel because we need to fix some bugs, implement InputEvent.getTargetRanges(), and finally, we need to contact websites which are broken by firing beforeinput event. The pref will be dom.input_events.beforeinput.enabled.

Comment 1

[:Harald Kirschner :digitarald]

See bug 1588007 for a patch that added the fetch event.

Comment 2

[theprover]

Hello Masayuki. I am a new contributor here. Can I work on this bug?

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to theprover from comment #2)

Hello Masayuki. I am a new contributor here. Can I work on this bug?

I'm not familiar with the code of DevTools, Harald, could you let us know who is a good mentor for this bug?

Comment 4

[Brian Birtles (:birtles, Feb 26~ parental leave)]

CC-ing Logan since he knows the event breakpoints code and might be able to suggest a mentor.

Comment 5

[Logan Smyth [:loganfsmyth]]

I'd be happy to help review this patch. I'd expect it to be relatively straightforward, based on the patch for the issue Harald linked above: https://phabricator.services.mozilla.com/D52297 The main difference is that we'll need to make the entry in the breakpoint list conditional based on Services.prefs.getBoolPref("dom.input_events.beforeinput.enabled") (I assume)

Comment 6

[:Harald Kirschner :digitarald]

Sorry for not clearing my ni? earlier – are you still interested in working on this, theprover ?

Comment 7

[Farooq AR]

Hi, I am a new contributor here. I'd like to explore this one and work on it if theprover is not working on it.

Comment 8

[Farooq AR]

Attachment: Bug 1607686 - Add beforeinput to Event Listener Breakpoints. r=loganfsmyth

Comment 9

[Farooq AR]

Attachment: Bug 1607686 - Add beforeinput to Event Listener Breakpoints. r=loganfsmyth

Comment 10

[Farooq AR]

Attachment: Bug 1607686 - Add beforeinput to Event Listener Breakpoints. r=loganfsmyth

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6870283c9949
Add beforeinput to Event Listener Breakpoints. r=loganfsmyth

Comment 12

[Alexandru Michis [:malexandru]]

Backed out changeset 6870283c9949 for causing xpcshell failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/11a4293973dceef0cc74e5b4d28cad58603463a8

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=f7617bd606bcefd637033cf067dc3926f45ae398&searchStr=linux%2C18.04%2Cx64%2Casan%2Copt%2Cxpcshell%2Ctests%2Ctest-linux1804-64-asan%2Fopt-xpcshell-e10s-4%2Cx%28x4%29&tochange=11a4293973dceef0cc74e5b4d28cad58603463a8&selectedJob=289189969

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=289189969&repo=autoland&lineNumber=3039

Comment 13

[Logan Smyth [:loganfsmyth]]

Sorry about that Farooq, looks like the failure is

Unexpected exception TypeError: can't access property "getBoolPref", Services.prefs is undefined at resource://devtools/server/actors/utils/event-breakpoints.js:162

It looks like Services.prefs is not available in Workers, so we'll want to make sure Services.prefs is truthy first, e.g.

      Services.prefs && Services.prefs.getBoolPref("dom.input_events.beforeinput.enabled")
        ? generalEvent("keyboard", "beforeinput")
        : null,

since beforeinput isn't an issue in workers anyway, we can just ignore that case.

Comment 14

[Farooq AR]

Will do that. On a side note, is there a recommended way to check these kind of errors beforehand?

Comment 15

[Alexandru Michis [:malexandru]]

Also seeing dt failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&fromchange=0f628062e4f0c25f0a068b64c4f19cd8b7ec4fcc&searchStr=%28dt&selectedJob=289196007

Yes, you can push to try the jobs that you think might fail, here is how: https://firefox-source-docs.mozilla.org/tools/try/selectors/index.html

Comment 16

[Farooq AR]

Attachment: Bug 1607686 - Add beforeinput to Event Listener Breakpoints. r=loganfsmyth

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8415a5f0b197
Add beforeinput to Event Listener Breakpoints. r=loganfsmyth

Comment 18

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/8415a5f0b197