Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 375058

Summary: Allow Command keybindings to work inside text fields
Product: [ECD] Orion Reporter: Mark Macdonald <mamacdon>
Component: ClientAssignee: Susan McCourt <susan>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: libingw, mamacdon, Silenio_Quarti, susan
Version: 0.5   
Target Milestone: 1.0 M2   
Hardware: PC   
OS: Windows 7   
Whiteboard:

Description Mark Macdonald CLA 2012-03-22 10:32:51 EDT
Orion I20120321-2230

For bug 374922, I prototyped a keybinding for the "Save" command in the site editor. I bound it to Ctrl+S.

However, by design the CommandService ignores bindings while focus is inside a text field. This meant that Ctrl+S triggered the unwanted browser 'Save As' dialog whenever you were editing a field -- and while using the site editor, you are often inside a field.

If the intent is to avoid clobbering platform keys for text fields (eg. Home, End, Ctrl+A) then perhaps the CommandService could be more selective in filtering so stuff like Ctrl+S is allowed?
Comment 1 Susan McCourt CLA 2012-03-22 11:22:05 EDT
(In reply to comment #0)
> Orion I20120321-2230
> 
> For bug 374922, I prototyped a keybinding for the "Save" command in the site
> editor. I bound it to Ctrl+S.
> 
> However, by design the CommandService ignores bindings while focus is inside a
> text field. This meant that Ctrl+S triggered the unwanted browser 'Save As'
> dialog whenever you were editing a field -- and while using the site editor,
> you are often inside a field.
> 
> If the intent is to avoid clobbering platform keys for text fields (eg. Home,
> End, Ctrl+A) then perhaps the CommandService could be more selective in
> filtering so stuff like Ctrl+S is allowed?

The intent was to prevent the command service from interpreting stuff like "?" or "T" when it should just be a character.  It does seem like we could/should be more selective here, we could ignore anything that doesn't have a mod key.
Comment 2 Susan McCourt CLA 2012-08-16 11:35:14 EDT
This bug is driving me nuts lately.  I'm always using ctrl+shift+F while already inside the ctrl+F slideout.  

Talked to Silenio.  It seems there's no way for us to know if a dom element natively handles some key combination (such as ctrl+A, or the cut/copy/paste keystrokes, etc.).

So to fix this we'd have to propagate anything that's not a character, and assume the client will not have conflicting bindings.  In other words, if someone defined a ctrl+A binding that was global to the page, and focus was inside a text field, then ctrl+A would do both "select all" and whatever the global binding was.

If we run into this problem in practice (for example, there are legitimate uses of ctrl+A in navigators, etc.) then we could ensure the bindings are scoped to the right level to avoid conflict.

This is what I'll pursue.
The trick remaining then, is to determine when a keydown event is actually a character.  (The browser will send a following keypress when it *is* a character, but we need to determine up front when it's *not* a character so we can handle the keypress).  Silenio has pointed me to code in StyledText (eclipse desktop) that does this same determination, I'll start with that.
Comment 3 Susan McCourt CLA 2012-08-16 15:27:20 EDT
Fixed in 
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=2d66e300ef09f501a09d6ac37e0d1f4bb71bb237

Two issues.  The first, as described, is to detect non-content modifying keys and allow those to propagate.  So now we detect characters, as well as content traversal or modifying keys like home/end, bksp/del, etc. and if a key *not* content modifying or content traversing, we let it bubble.  This fixed the problem whereby most global bindings (ctrl+shift+F, ctrl+shift+M) now work from a slideout.

The specific case of using Ctrl+Shift+F of the editor was still not working.  After a big of debugging I found that the textSearcher slideout was hooking keys so it could detect Ctrl+F and then canceling the bubble.  Instead what we need to do is check that *only* Ctrl+F is pressed, so that Alt+Ctrl+F and Shift+Ctrl+F can still bubble.  So I changed the key handling in textSearcher to check "Ctrl only" instead of just for the presence of Ctrl.  cc'ing Libing...can you look at the commit and sanity check that this is the right fix?

I think Mark's case as reported is no longer valid since the site editor auto-saves.