Open Bug 407037 Opened 14 years ago Updated 13 years ago

Beep for no-such-shortcut is missing for some key combinations

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set
minor

Tracking

()

REOPENED

People

(Reporter: jruderman, Unassigned)

Details

(Keywords: regression)

Cmd+Shift+K no longer beeps
Cmd+Shift+O no longer beeps
Cmd+Shift+D still beeps

I think this is a regression from bug 376077, "Cmd-enter in Location Bar beeps but does correct action".
Oh, Cmd+Shift+D is "bookmark all tabs".  So I guess the beep for disabled shortcuts still works, but the beep for no-such-shortcut is missing.
I backed out 407037, this should be fixed. Please leave this as blocking that bug so I can see this when I re-work the fix for that.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Sorry, I backed out 376077 not 407037.
The beeping can only happen for key equivs that are *in* the native menu system but are disabled. If something isn't in the menu system we can't know if it was handled and therefore we can't beep. If we want to change that, it will have to be on a command-by-command basis, outside of Cocoa widgets. Whether or not thing are actually working like this at any given time is a separate issue, but that is how things should work theoretically afaik.

My reading of this bug report suggests that this isn't really a bug, because that behavior is in line with what I just wrote. Cmd+Shift+K and Cmd+Shift+O are not in the native menu system, therefore they will never beep. Cmd+Shift+D beeping works because it is in the native menu system. I believe the confusion here stems from the fact that our somewhat broken key handling wrt commands like this was beeping for things for which it couldn't possibly know whether they were handled or not. Like bug 376077, which this was filed in response to. As we shift towards the correct behavior that I outlined above, I suspect there will be more confusion over this but the current architecture of Gecko doesn't allow for the optimal command-beeping behavior.
Resolution: FIXED → INVALID
(In reply to comment #4)
> The beeping can only happen for key equivs that are *in* the native menu system
> but are disabled.

... in Minefield. This works fine in native apps.

(Just wanted to make the distinction that this bug was closed even though it was asking for parity with native apps. Personally, I think that if this is still true, even if there are technical limitations, it should probably remain open until such a time as we can fix it like other native apps.)
What Sam said: "current technical limitations" is a reason to leave a bug open, not to mark it as wontfix/invalid.

As for bug 376077, why can't we fix it the way I suggested in 
bug 376077 comment 5?  Turning off beeping globally seems like overkill.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
No longer blocks: 376077
Assignee: joshmoz → nobody
You need to log in before you can comment on or make changes to this bug.