Bug 14140 – REGRESSION: Complex system KeyBindings don't work properly

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Michail Pishchagin]

$ cat ~/Library/KeyBindings/DefaultKeyBinding.dict
{
        "^m" = {
                "^s"    = ("insertText:", "\U21E7");  /* C-s     shift */
        };

        "~+" = ( "cut:", "insertText:", "«", "paste:", "insertText:", "»", "moveBackward:" );

        /* HTML/XML opening and closing tag pair. */
        "^<" = ( "setMark:",
                 "moveWordBackward:",
                 "deleteToMark:",
                 "insertText:", "<",
                 "yank:",
                 "insertText:", ">",
                 "setMark:",
                 "insertText:", "</",
                 "yank:",
                 "insertText:", ">",
                 "swapWithMark:" );
}

When entering text in text areas, these commands don't work properly in Safari 3 anymore. In Safari 2 I used to type "a", then press 

&#8963;&#8679;<, and "a" got automatically converted to "<a></a>" with cursor flashing in the middle. Now it's no longer the case. Also "&#8997;+" doesn't result in "«»" appearing.




Also "&#8963;M &#8963;S" doesn't insert "&#8679;" anymore, seems that nested bindings are no longer handled at all.

[Michail Pishchagin]

Shortcuts haven't displayed as intended, I've used mnemonics to describe them instead:

These commands don't work properly in Safari 3 anymore. In Safari 2 I used to type "a", then press "Ctrl-Shift-<", and "a" got automatically converted to "<a></a>" with cursor flashing in the middle. Now it's no longer the case. Also Opt-+ doesn't result in "«»" appearing.

Also "Ctrl-M Ctrl-S" doesn't insert shift key glyph anymore, seems that nested bindings are no longer handled at all.

[Mark Rowe (bdash)]

Can you attach your entire DefaultKeyBinding.dict file to this bug to make testing easier?  This is almost certainly fallout from the move to engine-rendered form controls from NSViews. I think you'll find that your custom key bindings are similarly broken in Safari 2.0 when editing an "editable region" rather than a form control.

[Michail Pishchagin]

Created attachment 15028 [details]
~/Library/KeyBindings/DefaultKeyBinding.dict

[Mark Rowe (bdash)]

I can confirm that with your key bindings file C-m C-s does not insert the Shift glyph.  When I type "a", then press C-S-<, I see the following in my text field:
a><</>

This is definitely less than desirable.  I was testing with a recent WebKit build on Leopard.

[Mark Rowe (bdash)]

<rdar://problem/5270958>

[Alexey Proskuryakov]

Regression -> P1

[Alexey Proskuryakov]

Created attachment 17932 [details]
proposed fix

That's a nice feature, I didn't even know about it!

Unfortunately, there doesn't seem to be a way to write a layout test - I couldn't find any API to install a per-application key binding dictionary. Xcode seems to do that somehow, presumably with an SPI.

[Darin Adler]

Comment on attachment 17932 [details]
proposed fix

+        Vector<KeypressCommand>& keypressCommands() { return m_keypressCommands; }

Even in a case like this, I usually prefer to provide a get/set pair rather than a single accessor. This allows us to change the set operation to do additional work without changing all callers, for example.

Change looks good, r=me

[Alexey Proskuryakov]

Committed revision 28772.

I haven't taken the suggestion to use a get/set pair: there aren't a lot of callers in this case, so it would be easy to modify this when needed. Also, I'm not sure if KeyboardEvent is even the right object to store this information.