Closed Bug 1154182 Opened 9 years ago Closed 9 years ago

OSX keybindings don't work in remote iframes (b2g desktop)

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla40
Tracking Flags:
Tracking Status
firefox40 --- fixed

People

(Reporter: paul, Assigned: paul)

References

Details

Attachments

(2 files, 1 obsolete file)

v1
1.79 KB, patch
Details | Diff | Splinter Review
v1.1
2.20 KB, patch
billm
: review+
Details | Diff | Splinter Review 
alt backspace: should delete a word
command left: should move curret to the begining of the line
command right: should move curret to the end the line
command shift: right should select text to the end of line
command shift: left should select text to the begennig of line
command c: copy
command v: paste
command x: cut

An easy way to reproduce is to enable remote tabs in B2G desktop:

> user_pref("dom.ipc.tabs.disabled", false);

then open Gaia's browser and try to copy some text.
I'm willing to work on this if it's not too hard and someone can assist me.
I'm wondering why Option+Key works when Cmd+Key doesn't.
Can you help?
Flags: needinfo?(mstange)
With: NSPR_LOG_MODULES=NativeKeyBindings:5 NSPR_LOG_FILE=/tmp/gecko.log

Notes:
- /tmp/gecko.log.child-1 is empty.
- in child process, we see 3 dumps instead of 1


Opt+Left in the parent process (work as expected):


2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, selector=moveWordLeft:
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, command=cmd_wordPrevious
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, handled=true


Cmd+Left in the parent process (work as expected):


2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, selector=moveToLeftEndOfLine:
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, command=cmd_beginLine
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, handled=true


Opt+Left in the remote iframe (work as expected):


2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, selector=moveWordLeft:
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, command=cmd_wordPrevious
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, handled=true
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, selector=moveWordLeft:
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, command=cmd_wordPrevious
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, handled=true
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, selector=moveWordLeft:
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, command=cmd_wordPrevious
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, handled=true


Cmd+Left in the remote iframe (doesn't work):


2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, selector=moveToLeftEndOfLine:
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, command=cmd_beginLine
2049012480[10039a070]: 11af25940 NativeKeyBindings::KeyPress, handled=true
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, selector=moveToLeftEndOfLine:
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, command=cmd_beginLine
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, handled=true
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, interpreting
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, bindingCommands=1
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, selector=moveToLeftEndOfLine:
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, command=cmd_beginLine
2049012480[10039a070]: 10034e550 NativeKeyBindings::KeyPress, handled=true
Also, emacs-like keybindings fail too (like Ctrl-a / Ctrl-e).
As far as I understand, commands should be forwarded to the child process. But copy and paste commands are supposed to be forwarded via `copypaste-docommand`. Investigating.
Clipboard commands are not forwarded because of an exception in docshell.isCommandEnabled: NS_ERROR_FACTORY_NOT_REGISTERED: Component returned failure code: 0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED) [nsIDocShell.isCommandEnabled]
editor/libeditor/nsPlaintextDataTransfer.cpp
   nsCOMPtr<nsIClipboard> clipboard(do_GetService("@mozilla.org/widget/clipboard;1", &rv));
   NS_ENSURE_SUCCESS(rv, rv);

This is where the exception comes from.
This is not a final patch. It just illustrates what goes wrong.

There's not clipboard component for the B2G content process on Desktop. This patch enables nsClipboardProxyConstructor.

I don't know how it works with B2G on Gonk or Desktop Firefox (e10s), so I'm not sure what the right approach is.
Flags: needinfo?(mstange)
Attachment #8592664 - Flags: feedback?
Attachment #8592664 - Flags: feedback?
Bill, apparently you worked on bug 966467. Can you help me figure out how to properly fix this bug?
Flags: needinfo?(wmccloskey)
For the second part of this patch, I need to forward commands to the child process. Or make it so commands like "cmd_beginLine" are triggered within the child process.

Forwarding from the main process appears to work (I used DoCommandHelper.handleEvent('beginLine')).
We bail out of PuppetWidget::ExecuteNativeKeyBinding if B2G.
Removing the #ifdef in ExecuteNativeKeyBinding solves the keybinding issue.
Attached patch v1 — — Splinter Review 
This patch enables native keybindings and register the clipboard component for B2G on Desktop.

Let's land that once we landed bug 1121217.
Assignee: nobody → paul
Attachment #8592664 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Flags: needinfo?(wmccloskey)
Blocks: graphene
Depends on: 1121217
Flags: needinfo?(fabrice)
Fabrice, can you land that in the larch branch?
Flags: needinfo?(fabrice)
Flags: needinfo?(fabrice)
Comment on attachment 8592734 [details] [diff] [review]
v1

Review of attachment 8592734 [details] [diff] [review]:
-----------------------------------------------------------------

I tend to think that the root error here is that we should use MOZ_WIDGET_GONK instead of MOZ_B2G. I see no reason to not support that on regular b2g desktop builds.
Flags: needinfo?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #19)
> Comment on attachment 8592734 [details] [diff] [review]
> v1
> 
> Review of attachment 8592734 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I tend to think that the root error here is that we should use
> MOZ_WIDGET_GONK instead of MOZ_B2G. I see no reason to not support that on
> regular b2g desktop builds.

Yeah, I was thinking of that too, but I'm afraid that would have some impact on the simulator. I can verify that though.
Attached patch v1.1 — — Splinter Review 
B2G Desktop seems to work well.

Let's see if tests pass: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0a80fe0a6c0
Attachment #8593173 - Flags: review?(wmccloskey)
Attachment #8593173 - Flags: review?(wmccloskey) → review+
Keywords: checkin-needed
No longer depends on: 1121217
https://hg.mozilla.org/mozilla-central/rev/e924011f8244
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: