Closed Bug 972420 Opened 8 years ago Closed 8 years ago

[e10s] Clumsy Bird page scrolls down when pressing space bar

Categories

(Firefox :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 30

People

(Reporter: ttaubert, Assigned: billm)

References

Details

Attachments

(1 file)

Playing Clumsy Bird [1] in an oop window scrolls the page whenever I press the Space bar. This doesn't happen in a non-remote browser. I suppose there might be some problem with canceling the event? I couldn't figure out what the game does without taking a look at the libraries.

[1] http://ellisonleao.github.io/clumsy-bird/
Probably caused by bug 862519?
Depends on: 862519
I investigated this problem today. Here's what seems to be happening. I'm guessing (I haven't checked) that the page uses an onkeydown handler that returns false. The problem is that the scrolling behavior for the space key is triggered by a keypress event. Normally this works okay because we don't trigger keypress events if onkeydown returns false (see the isKeyDownCancelled behavior in [1]). But in electrolysis, the keydown event is sent straight to the content process and it can't return false in the parent process. So we still send the keypress event.

What do you guys think we should do here? One option is to link the keydown and keypress events with an ID. Then the code in TabChild could ignore keypress event if the corresponding keydown event returned false.

[1] http://mxr.mozilla.org/mozilla-central/source/widget/gtk/nsWindow.cpp#2966
No longer depends on: 862519
Flags: needinfo?(felipc)
Flags: needinfo?(bugs)
Hmm, interesting case.
I think we shouldn't send keypress events to child process, but puppet widget should generate them
if key down wasn't cancelled.
Flags: needinfo?(bugs)
I like that idea. I'm worried, though, that it will conflict with the patch in bug 862519. The nsXBLWindowKeyHandler usually handles keypress events. If we don't pass those on to the content process, how will it know it needs to send a reply?
(In reply to Olli Pettay [:smaug] from comment #3)
> Hmm, interesting case.
> I think we shouldn't send keypress events to child process, but puppet
> widget should generate them
> if key down wasn't cancelled.

I tried doing this, but the details of whether the NS_KEY_PRESS is created, and how it's created, are pretty platform-specific. That makes it hard to copy that logic into PuppetWidget. I'm not really sure what to do here.
Attached patch keypress-fixSplinter Review
This is actually pretty simple. It relies on there always being a keydown event before any keypress event. As far as I can tell, that's true. It also avoids any platform-specific issues.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8387313 - Flags: review?(bugs)
Flags: needinfo?(felipc)
Attachment #8387313 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/6c80860a2266
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.