Closed Bug 277463 Opened 20 years ago Closed 20 years ago

don't underline radio / checkboxes accesskeys if ui.key.menuAccessKey==0 (accesskeys should not be visible on Mac)

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.8beta1

People

(Reporter: jo.hermans, Assigned: asaf)

References

Details

(Keywords: platform-parity, regression)

Attachments

(2 files, 4 obsolete files)

Bug 68841 introduced underlining for access keys in XUL dialog boxes. That's
against the Apple Human Interface guidelines. We shouldn't do that on the Mac.
Should we even allow accesskeys in the GUI at all ?

Underlining inside an HTML page is bug 56701, and is different though.
-> Core/Keyboard: Navigation

> Should we even allow accesskeys in the GUI at all ?

Why not? This helps using Mozilla with the keyboard which is often faster than
with the mouse.
Assignee: jag → aaronleventhal
Component: XP Toolkit/Widgets → Keyboard: Navigation
QA Contact: jrgmorrison
(In reply to comment #1)
> -> Core/Keyboard: Navigation
> 
> > Should we even allow accesskeys in the GUI at all ?
> 
> Why not? This helps using Mozilla with the keyboard which is often faster than
> with the mouse.

It's against the Apple Human Interface Guidelines, which doesn't use access keys
  for checks boxes and radio boxes. They're sometimes used for accessing buttons
(Cmd-D for 'Don't Save' for example, but that's not supported by Mozilla
anyway), but they're not visible in the interface. The point is that the
interface should be as clean as possible for common users (using the mouse),
allow users with disablities (see below), but still hide shortcuts for advanced
users. I agree that these last ones aren't discoverable unless you read the Help.

Anyway, bug 187508 was recently checked in, and should allow access to the
controls with ctrl-tab, arrow keys and the spacebar. But it only works if you've
changed the System Preferences. This allows access for people that can't control
a keyboard, but uses sticky keys, a pointing stick, a joystick, voice control,
etc ...

Supporting the Apple's accessibility API is bug 157209
Flags: blocking1.8b?
Flags: blocking-aviary1.1?
Note: some of the accesskeys are expected to work, but they should *never* be
visible on mac.
Keywords: pp, regression
taking,-> 1.8b.
Assignee: aaronleventhal → bugs.mano
Target Milestone: --- → mozilla1.8beta
Attached patch patch (obsolete) — Splinter Review
I'm not sure why we're hardcoding accesskeys for checkboxes/radios but here's
it.
Attachment #170715 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #170715 - Flags: review?(mconnor)
While I'm on the subject I thought I'd just comment that the checkbox/radio
access key code doesn't account for intl.menuitems.alwaysappendaccesskeys
Comment on attachment 170715 [details] [diff] [review]
patch

please ignore the toolkit part
Attachment #170715 - Flags: review?(mconnor) → review?(aaronleventhal)
Attached patch toolkit patch (obsolete) — Splinter Review
Attachment #170764 - Flags: review?(mconnor)
Attachment #170764 - Flags: review?(mconnor) → review+
Attachment #170715 - Flags: review?(aaronleventhal) → review+
Attachment #170715 - Attachment is obsolete: true
Attachment #170715 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #170715 - Flags: review+
Attachment #170847 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #170847 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 170847 [details] [diff] [review]
patch for seamonkey (follow ui.key.menuAccessKey)

>+            var prefs = Components.classes["@mozilla.org/preferences-service;1"].getService
>+                                                    (Components.interfaces.nsIPrefBranch);
The correct way to wrap this is to break just after the ] and line up the
.getService with the .classes

>+            if (!/Mac/.test(navigator.platform))
>+              this.setAttribute("underlineAccesskey", true);
This needs to be a field, not an attribute (and you can init the field with
!/Mac/.test(navigator.platform) directly).
Attachment #170847 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #170847 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #170847 - Flags: review-
note to self: an example of underlined XUL accesskeys: Prefs > Web Features panel.
Attached patch patch for seamonkey (obsolete) — Splinter Review
Attachment #170847 - Attachment is obsolete: true
Attachment #170896 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #170764 - Attachment is obsolete: true
Attachment #170764 - Flags: review+
Comment on attachment 170896 [details] [diff] [review]
patch for seamonkey

Consider an != 0 on the pref just to clarify that the field is a boolean. Also
xml doesn't need a space before /> (handily text.xml is the only binding
document with extra spaces, so at least you're consistent!)
Attachment #170896 - Flags: review?(neil.parkwaycc.co.uk) → review+
Comment on attachment 170896 [details] [diff] [review]
patch for seamonkey

with neil's comments fixed.
Attachment #170896 - Flags: superreview?(bzbarsky)
Comment on attachment 170896 [details] [diff] [review]
patch for seamonkey

sr=bzbarsky if you init the field with !/Mac/.etc directly like Neil asked (and
drop the code in the catch in the constructor, of course).
Attachment #170896 - Flags: superreview?(bzbarsky) → superreview+
moving reviews.
Attachment #170967 - Flags: superreview+
Attachment #170967 - Flags: review+
Attachment #170896 - Attachment is obsolete: true
Attached patch toolkit patchSplinter Review
Attachment #170972 - Flags: review?(mconnor)
Summary: don't underline XUL accesskeys on Mac → don't underline radio / checkboxes accesskeys if ui.key.menuAccessKey==0 (accesskeys shoud not be visible on Mac)
Attachment #170972 - Attachment is obsolete: true
Attachment #170972 - Flags: review?(mconnor)
Summary: don't underline radio / checkboxes accesskeys if ui.key.menuAccessKey==0 (accesskeys shoud not be visible on Mac) → don't underline radio / checkboxes accesskeys if ui.key.menuAccessKey==0 (accesskeys should not be visible on Mac)
Comment on attachment 170972 [details] [diff] [review]
toolkit patch

oops, i touhjt it wasn't the right one, sorry for spam.
Attachment #170972 - Attachment is obsolete: false
Attachment #170972 - Flags: review?(mconnor)
xpfe patch checked in.
Attachment #170967 - Attachment description: address last comments → address last comments (checked in)
Attachment #170972 - Flags: review?(mconnor) → review+
Flags: blocking1.8b?
Flags: blocking-aviary1.1?
toolkit patch checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
I know this comment is a bit late because this has already been checked in, but
I think a better approach would have been to have added a preference for the
underlining of the GUI access keys and default the choice according to operating
system.  That way the application still works according to the human interface
guidelines for the GUI, but the choice of how the application works is given
back to the user.

I really don't care any more about how Steve Jobs thinks a computer application
should work than I do about what Bill Gates thinks.

Also, remember, these are called Human Interface Guidelines.  They are therefore
neither standards nor requirements.  

I would rather have the coice of how applications behave on my compter by mine.

Just my $.02.
read the summary again, the comment (ui.key.menuAccessKey is a pref).
Oops.  Sorry for that (and this) bugspam then.  Should have known you guys would
get it right.
Verified to be fixed in the latest trunk (after the toolkit patch
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050116
Firefox/1.0+

You can get the underlines back if you set ui.key.menuAccessKey to 17 (default
is 0). No need to restart. The easiest place to check is Preferences -> Web
Features.
Status: RESOLVED → VERIFIED
Component: Keyboard: Navigation → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: