redo binding is wrong, and reload hotkeys fails if chrome does not have focus

VERIFIED FIXED

Status

()

Core
Keyboard: Navigation
P4
normal
VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: dbaron, Assigned: Joe Hewitt (gone))

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [dogfood-][nsbeta3+]<blocked by some other bug>)

(Reporter)

Description

18 years ago
DESCRIPTION:  There should be a hotkey for reload, probably (Ctrl|Alt)-R, as in
4.x.  This is an important hotkey.  (My dad complained to me that it was missing.)

STEPS TO REPRODUCE:
 * hit Alt-R or Ctr-R

ACTUAL RESULTS:
 * nothing

EXPECTED RESULTS:
 * should reload page

DOES NOT WORK CORRECTLY ON:
 * Linux, mozilla, 2000-07-20-09-M17

WORKS CORRECTLY ON:
 * Linux NN 4.7x
(Reporter)

Updated

18 years ago
Keywords: 4xp, nsbeta3, polish

Comment 1

18 years ago
Are we using "Ctrl|Alt+R" for another command?  Chris, consult German and 
Ben, and find out what's going on here.  Let's fix or at least resolve this for 
beta 3.
Assignee: don → mcafee
Keywords: correctness
Priority: P3 → P2
Whiteboard: [b3nav+]
Target Milestone: --- → M20
yeah...i use this often in 4xp. imho, this is dogfood (dogfood for anyone who
writes web pages).
Keywords: dogfood

Comment 3

18 years ago
Putting on [dogfood-] radar.  Not critical to everyday use. 
Whiteboard: [b3nav+] → [b3nav+][dogfood-]
There is a strong precedent on Windows for F5 to be the refresh key too. Thus I 
would STRONGLY recommend that on Windows, both Ctrl+R and F5 cause a refresh.
*** Bug 14498 has been marked as a duplicate of this bug. ***

Comment 6

18 years ago

*** This bug has been marked as a duplicate of 14498 ***
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE
(Reporter)

Comment 7

18 years ago
Reopening.  This bug was marked a duplicate of a bug that only covered some of
the issues in it.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Comment 8

18 years ago
Nav triage team: [nsbeta3+]
Whiteboard: [b3nav+][dogfood-] → [dogfood-][nsbeta3+]

Updated

18 years ago
Priority: P2 → P1

Comment 9

18 years ago
recommend that we take this off the nsbeta3+ list at this point and ask for help 
from mozilla contributors.  removing [nsbeta3+], adding helpwanted

Looks like we will have to rely on other folks in mozilla to take on keyboard 
shortcuts.
Keywords: helpwanted
Whiteboard: [dogfood-][nsbeta3+] → [dogfood-]
akkana, timeless or blake --any of you have time for this? or know of someone
else who would?

Comment 11

18 years ago
I've already done this once for F5.
Assignee: mcafee → timeless
Status: REOPENED → NEW

Comment 12

18 years ago
yes, F5 is implemented.  is that good enough to close this?
(Reporter)

Comment 13

18 years ago
Absolutely not.  F5 is not indicated on the menus, and only windows users might
guess that it should work.  Ctrl/Alt-R is the Netscape standard and has been for
years.
(Reporter)

Comment 14

18 years ago
F5 doesn't work on Linux either.  (And it shouldn't, since it's a
Windows-ism...)

Comment 15

18 years ago
Erm iirc F5 works on ie solaris. I'll check next weekend.

As for <xulkey>-r sure I can do it. I was merely making a general comment while 
I accepted the bug.

Comment 16

18 years ago
Erm iirc F5 works on ie solaris. I'll check next weekend.

As for <xulkey>-r sure I can do it. I was merely making a general comment while 
I accepted the bug.
Status: NEW → ASSIGNED
btw, F5 doesn't work on linux. i agree with dbaron --Ctrl+R/Cmd+R is an
established (and easy to remember :) shortcut for reloading, and should be
implemented in mozilla/nscp6.

Comment 18

18 years ago
Does anyone have a problem with both F5 and Ctrl+R being used for reload (on 
win32, at least)?

Comment 19

18 years ago
It should be Ctrl-R, Cmd-R and Alt-R on each of the platforms respectively. If we 
can also support F5 on Win 32 - great, but Ctrl-R is the highest priority.

Comment 20

18 years ago
OK.  timeless, I'll fix this if you don't want it.
(Assignee)

Comment 21

18 years ago
I've got a fix for this. Blake, if you haven't fixed it already I'll check it 
in today.

Comment 22

18 years ago
I never got around to this.  I don't know if timeless did; I'm sure he doesn't 
mind if you go ahead and fix this, but it's probably a good idea to ask him 
beforehand.
Keywords: helpwanted
(Assignee)

Comment 23

18 years ago
Just checked in a fix for the hotkeys... should work now, as well as display 
the accelerator in the menu

Comment 24

18 years ago
Using 2000-09-08-06/Linux it doesn't work.

In view there is "Reload     R" instead of "Reload  Ctrl+R".
R and Ctlr-R don't work, F5 doesn't work either (should it work? It wasn't clear
to me from reading this bug.)

I just test Alt+R; it works; all other shortcuts in the menus (back, forward
etc.) use Ctrl.

Bugs:
a) Ctrl instead of Alt should be used (if the other do)
b) if Alt is used, it should appear in the menu.

My guess is that the with <foo>-r, the foo is wrong (foo = <xulkey> ?)

Comment 25

18 years ago
It was checked in explicitly using alt= for Unix, not the xulkey.  I'm curious
why it was checked in separately for the three platforms -- why not just put the
binding in once for the three platforms, with xulkey=, and let the normal xulkey
handler decide what the modifier is, like we do with all the other bindings?
(Assignee)

Comment 26

18 years ago
Two reasons why I did it that way:

1. xulkey was evaluating to Ctrl on Linux
2. There are separate versions of platformNavigationBindings.xul for each 
platform, and it seemed to belong in that file.
(Assignee)

Comment 27

18 years ago
Tobias, it says in this bug report that Alt-R is the correct hotkey for Linux, 
which is why I didn't implement Ctrl-R.  I noticed that the menu only says R, 
but I assume that is a seperate bug because the keystroke is hooked up 
correctly to Alt-R.

Comment 28

18 years ago
I think German was thinking about the 4.x bindings when he said alt, not about
the mozilla world with configurable xulkey; I think he just meant it should be
on the xulkey for all platforms.  Why wouldn't it be?  That would be confusing
to users, to have one key on alt when all the rest were on control.

Seems like since this is the same letter on all platforms, it should be on the
xulkey and live in the xp bindings file.  It's hard to maintain separate
platform bindings unless there's a definite need for them to be separate, and if
it's in the xp file, other platforms besides the "big three" will inherit it,
which they won't if it only lives in the platform files.
(Assignee)

Comment 29

18 years ago
Ok, I guess I misunderstood German.  I'll change it to xulkey and put it in 
navigatorOverlay.xul

Comment 30

18 years ago
Undoing johng [hewitt is doing the work for us, and I think it's only fair to 
keep the status], marking all/all
xulkey+r should cause reload and should appear correctly in the menu.
having F5 work on platforms where people don't object to it will be done later.

Thanks Joe for working on this, I'm sorry about the confusion, I should have 
called german on alt-r.
OS: Linux → All
Hardware: PC → All
Whiteboard: [dogfood-] → [dogfood-][nsbeta3+]

Comment 31

18 years ago
PDT thinks this is a P4
Priority: P1 → P4
Whiteboard: [dogfood-][nsbeta3+] → [dogfood-][nsbeta3+][PDTP4]
(Assignee)

Comment 32

18 years ago
Ok, this one should be fixed now.
the shortcut (eg, Ctrl+R) now appears in the main menu, but using the shortcut
doesn't actually reload the page. testing on linux, 2000.09.13.08 opt comm bits.
anyone else experience this (or is making it work (backend) a seperate bug?)?
(Assignee)

Comment 34

18 years ago
I noticed immediately that Ctrl-R didn't work on Windows... but then I 
discovered that it _does_ work when something in the chrome has the focus.  
When the browser has the focus, it does not work...  it should, though, because 
the keys are setup correctly.

Comment 35

18 years ago
Removing polish. Since this was marked nsbeta3.
Hewitt did you try a dump('reach'); to see if the code is being triggered?
Se/Hewitt if it isn't triggering please file a blocking bug and copy over the 
status whiteboard from here (minus <>) then remove <> and mark this as a 
dependency.

Thanks for working on my bug. fwiw, I don't remember having this problem w/ my 
version of the patch. I'll look into this after UI freezes.
Keywords: polish
Summary: reload should have a hotkey → reload's hotkeys should work
Whiteboard: [dogfood-][nsbeta3+][PDTP4] → [dogfood-][nsbeta3+][PDTP4]<blocked by some other bug>

Comment 36

18 years ago
resummarizing, I think we could minus this.
Summary: reload's hotkeys should work → reload hotkeys fails if chrome does not have focus

Comment 37

18 years ago
timeless says hewitt's working on this, over to him.

Assignee: timeless → hewitt
Status: ASSIGNED → NEW
(Assignee)

Comment 38

18 years ago
The reason that Ctrl-R fails to work is because there is a conflicting key 
defined in xpfe\communicator\resources\content\browserBindings.xul (line 68).  
This key is assigned to the redo command.  I'm not sure why Ctrl-R was assigned 
to redo, especially when it is indicated that Ctrl-Y is the redo hotkey in the 
Edit menu.  I could easily just change the "r" to a "y" in browserBindings.xul, 
but I'm not sure if there is a reason why it was there in the first place.  
Anyone have any ideas about this?
the main catch there is that Redo is Command+Shift+Z on Mac OS, but Ctrl+Y on
win32. i'm not sure what the Redo accelerator should be on linux (akkana,
mcafee?), but it's currently set to Ctrl+Shift+Z...

Comment 40

18 years ago
The redo accelerator should probably be the same on linux as it is on windows,
whatever that might be.  I tried several windows apps on my machine and couldn't
find any app that had multi-level undo, and the ones that have single-level undo
all use ^Z to toggle between undo and redo (i.e. the second undo undoes the
first undo), so I sent mail to editoreng, german and jglick hoping that someone
who knows what windows users expect would comment on the situation.  I don't
have a clue myself, having never used an app that uses multi-level undo (except
vim, and whatever the binding is there, it's probably not something that windows
users would like. :-)

Comment 41

18 years ago
redo is supposed to be bound to ctrl-y, for all platforms
that I am aware of.  German says redo "undoes whatever undo did".
We definitely shouldn't block reload, I think we should make the
change to browserBindings.xul as hewitt suggests.
Adding lake who published latest key bindings spec.
*** Bug 52880 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 43

18 years ago
Microsoft Apps such as Word use Ctrl-Y for redo.  This sucks, in my opinion.  I 
use Allaire Homesite for most text editing, and it's redo hotkey is Ctrl-Shift-
Z which complements Ctrl-Z nicely and prevents me from having to reach for the 
Y when toggling between undoes... this should probably be a separate bug.
Status: NEW → ASSIGNED

Comment 44

18 years ago
Joe is right -- Ctrl+R is *not* a Windows standard key. Ctrl+Y is, but that is 
fairly stupid. I'd also prefer Ctrl+Shift+Z instead.

Comment 45

18 years ago
PSP6 uses ctrl-alt-z for multi level undo.

We should fix ctrl-r first, and worry about it's replacement later.

Comment 46

18 years ago
agree with timeless.
Ctrl-shift-z already shows up in the Edit menu and seems to work for me,
I like it.  We should go fix the ctrl-r problem now.

Comment 47

18 years ago
It's pretty clear that we should change the redo binding, either to ^Y or
[ctrl][shift]Z.  If it's [ctrl][shift]Z, then it will be the same as Mac and we
could just put the binding in browserBindings.xul and remove all redo bindings
from the various platformBrowserBindings.xul files, and we'd have consistency
across platforms.  I'd suggest doing that unless someone strenuously argues for
^Y on Windows instead.
Summary: reload hotkeys fails if chrome does not have focus → redo binding is wrong, and reload hotkeys fails if chrome does not have focus

Comment 48

18 years ago
^Y has a strong precedence on windows, all Microsoft applications with
multi-level undo use it.  I suggest we do as well, unless there's some reason
not to.
This issue, whatever the decsion, should be P2 based on high value and low risk.
 What I mean is:  it should be P2 that we have no conflicting key bindings.  I
only have a mild preference for ^Y over Ctrl+Shift+Z.
Removed [PDTP4] for reconsideration of priority, for at least the trivial part
of this bug.
To what other bug does "<blocked by some other bug>" refer?  It should be listed
as a dependancy.
Whiteboard: [dogfood-][nsbeta3+][PDTP4]<blocked by some other bug> → [dogfood-][nsbeta3+]<blocked by some other bug>

Comment 49

18 years ago
I would recommend xulkey+R for reload and xulkey+Y for redo.

Comment 50

18 years ago
I'd recommend that we go with ctrl-shift-Z on Linux, as on Mac, then, rather
than following Windows, since it sounds like even on Windows, the only apps
which use ctrl-Y are Microsoft apps (which obviously don't exist on Linux) and
all the non-MS users who have responded seem to prefer ctrl-shift-Z (which
certainly is easier to remember and more intuitive in that it does the opposite
of ctrl-Z).

Might be cleanest to make an XP binding for ctrl-shift-Z, then in the windows
platform file, cancel that and put in a binding for ctrl-Y.  But it's your call.

Comment 51

18 years ago
For Reload the accelerator key is (appropriate platform key)+R.

Redo it something else altogether the function is different and they should not
share the same keys. Regardless of what redo is on different platforms it is not
R and so we should use that for Reload.

And stupid or not Ctrl Y is very standard on windows and keyboarders have
learned it and use it. Changing it would be the wrong move. Like changing paste
from V to P because it didn't make sense to you, people know it and depend on
it. Additionally though it shows up on some Mac applications the redo is not
specified in the Mac guidelines as a keyboard equivalent at all and is not
adopted by all applications for the mac. Yes some application for the Mac use
(Com+Y) including some non Microsoft apps! So Com+Shift+Z is not a given for the
Mac.

Reload is a high UI priority as well as redo. Redo is standard on Windows which
is has a large number of users. Reload is sepecially high in value for the
browser, some times it's the only way to get things to work properly.

R as Reload is a very high priority as it greatly effects the usability of the
browser.

Comment 52

18 years ago
And I forgot, for those who must use the keyboard it is importiant that the
often used things like a Redo use as few keys as possible. It is very difficult
for those with limited mobility to perform this function Mac apps have been
knocked for this in trade articles.

This will be a big deal next release when AOL and Netscape try to convince the
Govt. that they conform to ADD laws.
side note re: Redo: if we decide to use 'ctrl/cmd+y' instead of
'ctrl/cmd+shift+z', this will break bug 47677. cc'ing saari and brade who're on
that bug as well.

Comment 54

18 years ago
Yes it does. The feature Redo, was not implimented as speced and so we got bug 
47677 when the implimented stuff didn't work. The bug should have been "not 
implimented to spec and not working".

Comment 55

18 years ago
I'm really confused (please bear with me as I have been away for 6 weeks...)... 

In every UI spec I've seen for the Edit menu in mozilla or Netscape 6, there has 
always been a special callout or indiciation that Redo is to be command-shift-z 
on Macintosh.  Lake--what do you mean about "not implimented as speced"? 

Lake--what Macintosh applications use Command-Y instead of Command-Shift-Z?  (I 
haven't been able to find many applications that support multiple-level undo to
compare to)
(Assignee)

Comment 56

18 years ago
Ok, I re-assigned Ctrl-Y to redo in 
xpfe/communicator/resources/content/browserBindings.xul, and Ctrl-R should now 
work correctly for reload.  Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
vrfy fixed using 2000.09.29.xx-n6 [opt comm branch bits] on linux, mac and
winnt.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.