linux keybindings mimicking windows

VERIFIED FIXED in mozilla0.9

Status

()

Core
Editor
P3
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: sujay, Assigned: Akkana Peck)

Tracking

(Depends on: 1 bug, {pp, regression})

Trunk
mozilla0.9
Other
Linux
pp, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: workaround fix in hand)

Attachments

(4 attachments)

(Reporter)

Description

17 years ago
using 10/17 build of netscape

1) launch netscape
2) launch composer
3) enter some text
4) hit ^A

it should take you to home, but it doesn't. instead it selects all.

5) hit ^K

it should delete to the end of the line. but it doesn't. instead it
invokes the spellchecker!

Linux platform only.
(Assignee)

Comment 1

17 years ago
Apparently the order in which keybindings are evaluated changed recently: now xp
bindings take precedence over platform bindings.  I mentioned this to Saari
yesterday, and he thought he might know what changed.

This is a regression which will prevent Unix users from using their customary
editing keys.
Assignee: akkana → saari
Keywords: regression
hm...now that i think of it...would this be perchance a dup of bug 55408? if
not, then dependent on that bug?
(Assignee)

Comment 3

17 years ago
'Fraid not, though that bug would hide this one for people who used the default.

Updated

17 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9

Comment 4

17 years ago
Normally I don't like prefs, but in this case I think we do need one -- a 
choice between Windows-/Mac-style keybindings (so the hordes migrating from 
other OSes can continue using the keybindings they are accustomed to) and the 
traditional emacs keybindings (so Unix users can continue using the keybindings 
they are accustomed to).

Comment 5

17 years ago
Um, that's a packaging nightmare.  And if you do it, you'll really need to be 
fair and let the users of os Y select to use any binding available.  [Some poor 
mac user trapped at a windows box...]

The way i see this happening is by having an empty keybinding file and then 
having an rdf overlay the correct keybindings.  Most likely this will be done 
by marking keybindings as a separate package.

hyatt, ben, dveditz: comments?
Keywords: pp
(Assignee)

Comment 6

17 years ago
We basically already have this, by switching the pref to use alt keybindings.

What we don't have is a way to conditionally include sets of xbl bindings, to
get around the problem that there are conflicts when the access key is ctrl.  We
need that to un-mess-up the Unix binding situation.

Hyatt told me a while ago shipped that the chrome and resource scheme was being
revised and that it would be possible for users to select different xbl files,
something like the old mechanism that stopped working where an XBL file for
behavior could be specified in userChrome.css.  Hyatt, what's the status on
that, and is there a bug?  Will it eventually be possible for us to switch
between two different platformHTMLBindings.xbl files based on a pref or
userChrome setting or something?

Perhaps a temporary solution would be to put all key bindings in one separate
jar file (or keep them out of jars altogether) which would make it easy to offer
replacement files (users would still have to copy a file to change their
bindings, but it's better than unzipping/editing/rezipping).

Comment 7

17 years ago
The pref is bug 49909.
(Assignee)

Comment 8

17 years ago
Yes, that's the bug for adding a pref UI for the pref that switches the accel
key.  Changing individual key bindings is covered by bug 18508 (backend) and bug
57805 (make a UI for key bindings).

This bug is probably a dup of one of those, not sure which since it talks about
several issues.
*** Bug 65538 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Target Milestone: mozilla0.9 → mozilla1.0
I think the biggest issue here is consistency.  In textareas and form inputs,
C-a will go to beginning of line, C-h will delete a character.  In mailnews
compose and editor, C-a will select all and C-h will open browser history!

Adding dependencies on the C-a and C-h bugs.
Depends on: 59404, 68511
No longer depends on: 68511
Depends on: 68511
(Assignee)

Comment 11

17 years ago
Are people still seeing this?  I thought about poking around in the XUL key
code, tried changing my accel key back to ctrl and didn't see this problem --
ctrl-A always went to beginning of line, never did select-all.  If this is still
happening, when and where is it seen?
Still an issue for me in mailnews composition with a 2001-02-12 build
*** Bug 69230 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 14

17 years ago
Sure enough, I see it too now, in the editor and mail compose windows, but not
in text controls.  Looks like there's a problem with the way the XBL bindings
are getting registered only in toplevel windows.  I'll see if I can find out more.
Status: ASSIGNED → NEW
(Assignee)

Comment 15

17 years ago
This was previously marked as assigned, but it got changed when I made my last
comment.  Changing it back.

Meanwhile, I've been poking around reordering various lines in
nsXBLWindowHandler.cpp, but haven't had any luck with fixing this.
Status: NEW → ASSIGNED
(Assignee)

Comment 16

17 years ago
On Hyatt's suggestion, I looked at the XUL bindings.  Turns out that when we
switched over to XBL bindings for the editor and browser windows, the XUL
bindings files were never removed from the build, or the tree.

We should remove them -- both browserBindings.xul and editorBindings.xul.  Any
others?  We should remove them from the source tree entirely so they don't
mistakenly get added back in somewhere along the line (which is probably what
happened here).

I'm taking this because someone needs to do it.
Assignee: saari → akkana
Status: ASSIGNED → NEW
(Assignee)

Comment 17

17 years ago
Created attachment 26238 [details] [diff] [review]
Initial diff (breaks some bindings)
(Assignee)

Comment 18

17 years ago
Turns out there are still a lot of XUL key bindings which should be done in XBL.
I've attached a patch which removes all of them.  Now some things don't work:
1. The cut/copy/paste bindings don't show up in the menus any more (even though
they still work correctly).
2. "New blank page" and "New Navigator Window" no longer work -- I guess nobody
ever bothered to add XBL bindings for those functions?

Adding dr -- I think he had something to do with the switch from XUL to XBL --
and Ben for the xul bindings and the menu issue.  Can someone help with getting
the key binding legends in the menus fixed and with deciding where the accel-N
and accel-shift-N XBL bindings should live?

Marking moz 0.9 because users shouldn't have to put up with this any longer.
Target Milestone: mozilla1.0 → mozilla0.9

Comment 19

17 years ago
The key binding legends issue in menuitems is an interesting one.

The problem is that a <menu(item)/> refers to a <key/> to get the
modifier(s)+key information for the legend. I don't think there's currently a
way to get that information from xbl, so as a quick fix you could leave the
<key/>s around, just remove the oncommand attribute.
(Assignee)

Comment 20

17 years ago
Talked to Hyatt: there is currently no way to update the menus to show what the
current XBL bindings are.  Filed bug 71779 to cover that issue.  Meanwhile, jag
is probably right that we'll need to keep the xul files there to show bindings
in the menus, but remove the part that actually registers a key binding.  Yuck.
(Assignee)

Comment 21

17 years ago
I have a simpler patch, which keeps the <key> nodes but removes their observes
and oncommand attributes so they won't actually fire.

I'd like to get this in for 0.8.1, then address the harder problems and code
cleanup later.

Hyatt, any chance you could look at this and give an sr?  Kathy, could you
review it?  And if jag and/or blake would be willing to look it over as well, it
would help my confidence in this a lot.
Status: NEW → ASSIGNED
Whiteboard: fix in hand, awaiting r and sr
Target Milestone: mozilla0.9 → mozilla0.8.1
(Assignee)

Comment 22

17 years ago
Oh, great timing.  Bugzilla isn't accepting patches for some reason -- the
operation times out.  Until bugzilla is fixed, I've uploaded the latest
(smaller/safer) patch to: http://www.shallowsky.com/bugs/bindings.diff

Comment 23

17 years ago
(Actually, there is a mozilla bug in recent builds that prevented the upload of 
patches to bugzilla (if they above a certain size). A fix is forthcoming.)
bug 71346

Comment 24

17 years ago
sr=hyatt.

Comment 25

17 years ago
moving to mozilla0.9
Target Milestone: mozilla0.8.1 → mozilla0.9
(Assignee)

Comment 26

17 years ago
Created attachment 27844 [details] [diff] [review]
Smaller, simpler patch for 0.8.1
(Assignee)

Comment 27

17 years ago
Created attachment 27845 [details] [diff] [review]
Kathy Brade's cleanup of XBL files
(Assignee)

Comment 28

17 years ago
Back to 0.8.1 (with Beth's okay), since we have a reviewed and sr'ed patch ...
we'll be doing a bit more testing while waiting for a go-ahead to check in.
Target Milestone: mozilla0.9 → mozilla0.8.1
(Assignee)

Updated

17 years ago
Whiteboard: fix in hand, awaiting r and sr → fix in hand
Assuming that this is low risk enough for 0.8.1 I'm fine with getting it in (
please! )

Is this something that's going to hurt the mail/news landing?  If so can it wait
until after we actually branch?

Comment 30

17 years ago
I added the 03/15/01/ 17:12 and 03/15/01/17:13 patches to my Win32 source tree 
and noticed the following:

Good news:

  Arrow keys and it's variants still work in widgets and Composer.
  Cut/Copy/Paste/Undo/Redo all work in widgets and Composer.
  No more double undos with a single Ctrl-Z anymore.

Bad news:

  SelectAll (Ctrl-A) is busted
  Ctrl-Shift-Home/End are busted in widgets
  PageUp/Down (cmd_movePageUp/Down) and it's variants don't work in widgets

Comment 31

17 years ago
also note: in my cleanup (see attachment), the scrollPageUp and scrollPageDown 
commands should be *move*PageUp and *move*PageDown in the Windows (and presumably 
Linux?) version of platformHTMLBindings.xml.

also, I think that pageup, pagedown isn't handled for textareas and it should be 
(subject of another bug)
(Assignee)

Comment 32

17 years ago
Page up/down in textareas was broken before: bug 65124 covers that -- but we
should fix that with this checkin as long as we're fixing basic keybindings.

SelectAll: turns out there was no selectAll key binding in XBL before (!).
We should be able to add a selectAll binding for each set of bindings in
htmlBindings.xml, then let the unix/platformHTMLBindings.xml override that. 
Note the *should*: it turns out that platform bindings are not currently
overriding xp bindings.  (Full circle back to the original bug!)

So, to fix this on Unix in a way that doesn't remove the binding on windows/mac,
we have two options:
(1) add selectAll bindings to the mac and windows platformHTMLBindings.xml.
(2) add them to the XP bindings file, and fix the problem in XBL that is not
letting platform bindings override xp bindings.

We should fix the problem anyway, of course, but it may be too late to do that
for 0.8.1.  In other cases where two platforms use a binding and the third puts
something else on that key, we have taken approach (1), put the binding in the
platformHTMLBindings for the two platforms and kept it out of the xp bindings. 
(In fact, Kathy's cleanup moves more such bindings out of xp and into duplicate
platform bindings.)  So if we want a fix for 0.8.1, that's what I suggest we do,
consistent with our approach on other bindings.  It's four extra lines in
mac/platformHTMLBindings.xml and win/platformHTMLBindings.xml, and I'll attach a
new diff if anyone's interested.

Comments?
(Assignee)

Comment 33

17 years ago
*** Bug 59404 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 34

17 years ago
Created attachment 27939 [details] [diff] [review]
turn on selectAll for mac and windows
(Assignee)

Comment 35

17 years ago
I've attached a small patch to turn selectAll back on for mac and windows. 
Apply either instead of Kathy's cleanup diff, or on top of it (on top of it will
have different line numbers and might need to be applied by hand).  

Kin's report of ctrl-shift-home being broken in widgets on windows worries me,
though, because those bindings are there and look correct to me.  Maybe it's
another case like bug 65124, paging bindings mysteriously not working.
I did fixed a missing shift in the mac binding for shift-home.
Whiteboard: fix in hand → workaround fix in hand
(Assignee)

Comment 36

17 years ago
With help from Hyatt, I determined that the problem was actually that both the
platform and XP bindings were firing (and in the case of selectAll and
beginLine, you don't see the beginLine if there's a selectAll after it).  This
is because nsXBLPrototypeHandler::ExecuteHandler calls PreventDefault, but it
doesn't check GetPreventDefault before firing.  This patch cures that problem
and should make things much nicer for platform and user bindings which override
XP bindings.  (We still need the XUL diffs, though -- XUL bindings will still
win over XBL.)

Index: nsXBLPrototypeHandler.cpp
===================================================================
RCS file: /cvsroot/mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp,v
retrieving revision 1.17
diff -u -r1.17 nsXBLPrototypeHandler.cpp
--- nsXBLPrototypeHandler.cpp   2001/03/13 01:55:35     1.17
+++ nsXBLPrototypeHandler.cpp   2001/03/17 01:37:00
@@ -189,6 +189,14 @@
   if (!mHandlerElement)
     return NS_ERROR_FAILURE;
 
+  nsCOMPtr<nsIDOMNSUIEvent> uievent = do_QueryInterface(aEvent);
+  if (uievent) {
+    PRBool preventDefault;
+    uievent->GetPreventDefault(&preventDefault);
+    if (preventDefault)
+      return NS_OK;
+  }
+
   // This is a special-case optimization to make command handling fast.
   // It isn't really a part of XBL, but it helps speed things up.
   nsAutoString command;

This came from hyatt and so has sr, but needs an r= from someone else.

Comment 37

17 years ago
r=brade
(Assignee)

Comment 38

17 years ago
Giving up on 0.8.1, this just isn't getting much testing.
Target Milestone: mozilla0.8.1 → mozilla0.9
(Assignee)

Comment 39

17 years ago
I checked in my part of this a week ago; Kathy checked in her part last week, so
I'm marking this fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 40

17 years ago
Ctrl-A does take you to Home

but

Ctrl-K still brings up the spellchecker...it should delete to the
end of the line right ?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Is the spellchecker in mozilla builds?  Or is this a bugscape issue now?

Comment 42

17 years ago
spellchecker ui is in mozilla, netscape comm builds have an engine. the 
question is does our ui happen reguardless of the presence of an engine.
(Assignee)

Comment 43

17 years ago
Funny, for me in today's build, if I set accel=ctrl, ctrl-K works correctly
(deletes to end of line) but ctrl-H now brings up the history window instead of
deleting backward.
(Assignee)

Comment 44

17 years ago
A few specific keybindings have regressed.  Sujay has filed bug 77976 for those
keybindings, rather than morphing this more general bug.  Re-closing this one,
and we'll look at the ^K/^H issues in that bug.
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 45

17 years ago
verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.