Closed Bug 221783 Opened 17 years ago Closed 15 years ago

<wizard>'s buttons have no access keys

Categories

(Core :: XUL, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.8.1

People

(Reporter: Stefan.Borggraefe, Assigned: vhaarr+bmo)

References

(Blocks 1 open bug)

Details

(Keywords: fixed-seamonkey1.0, fixed1.8.1, polish)

Attachments

(2 files, 4 obsolete files)

The buttons of the <wizard> element currently have no access keys. There should
be at least some for the "Next" and "Back" buttons. The "Cancel" and "Finish"
button probably fall under the category "Common elements that don't get
accesskeys" listed here:

http://www.mozilla.org/projects/ui/accessibility/accesskey.html

They can already be accessed via Return and Escape respectively.
Attached patch toolkit version 0.1 (obsolete) — Splinter Review
Adds accesskey to all wizard buttons, including Finish and Cancel.
Attachment #196251 - Flags: review?(mconnor)
Attached patch xpfe version 0.1 (obsolete) — Splinter Review
xpfe version of attachment 196251 [details] [diff] [review].
Attachment #196252 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 196252 [details] [diff] [review]
xpfe version 0.1

Comment 0 was correct, Finish and Cancel do not have access keys. Also, please
use accesskey-<type> like dialog.xml does for its properties.
Attachment #196252 - Flags: review?(neil.parkwaycc.co.uk) → review-
Attached patch version 0.2 (obsolete) — Splinter Review
I presume you meant like this?
Attachment #196252 - Attachment is obsolete: true
Attachment #196601 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 196601 [details] [diff] [review]
version 0.2

Yes but you still need empty accesskey-finish= and accesskey-cancel= entries in
the .properties files.
Attachment #196601 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attached patch xpfe version 0.3Splinter Review
Neil: Since I'm not sure who you'd like to sr this, I thought you could
re-assign the sr request, if that's OK :-)

I'll attach an updated toolkit patch shortly.
Assignee: Stefan.Borggraefe → vhaarr+bmo
Attachment #196601 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #196643 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #196643 - Flags: review+
Attached patch toolkit version 0.2 (obsolete) — Splinter Review
toolkit version updated to Neils comments.
Attachment #196251 - Attachment is obsolete: true
Attachment #196644 - Flags: review?(mconnor)
Attachment #196251 - Flags: review?(mconnor)
Attachment #196643 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Whiteboard: [checkin needed] (attachment 196643)
Neil or Aaron: Could you help me check in attachment 196643 [details] [diff] [review]?

If your review is accepted for toolkit as well, maybe we can take attachment
196644 [details] [diff] [review] off mconnors radar?
Comment on attachment 196644 [details] [diff] [review]
toolkit version 0.2

Trying Mano for this, in case access key behaviour is platform-specific.
Attachment #196644 - Flags: review?(mconnor) → review?(bugs.mano)
Comment on attachment 196644 [details] [diff] [review]
toolkit version 0.2

(Note it is indeed disabled on OS X; We do allow to turn on accesskeys though.)

>Index: toolkit/content/widgets/wizard.xml
>===================================================================

>+             var accesskey;
> #ifdef XP_UNIX
> #ifdef XP_MACOSX
>-            btn.setAttribute("label", this._bundle.GetStringFromName("button-"+aName+"-mac"));
>+             btn.setAttribute("label", this._bundle.GetStringFromName("button-"+aName+"-mac"));
>+             accesskey = this._bundle.GetStringFromName("accesskey-"+aName+"-mac");
> #else
>-            btn.setAttribute("label", this._bundle.GetStringFromName("button-"+aName+"-unix"));
>+             btn.setAttribute("label", this._bundle.GetStringFromName("button-"+aName+"-unix"));
>+             accesskey = this._bundle.GetStringFromName("accesskey-"+aName+"-unix");
> #endif
> #else
>-            btn.setAttribute("label", this._bundle.GetStringFromName("button-"+aName));
>+             btn.setAttribute("label", this._bundle.GetStringFromName("button-"+aName));
>+             accesskey = this._bundle.GetStringFromName("accesskey-"+aName);
> #endif


decale and init |accesskey| inside the #ifdefs.


>+             if (accesskey) {
>+               btn.setAttribute("accesskey", accesskey);
>+             }

please remove single line brackets.

r=mano with those fixed.
Attachment #196644 - Flags: review?(bugs.mano) → review+
(In reply to comment #10)
> decale and init |accesskey| inside the #ifdefs.

I do not know what 'decale' means .. Spelling error for 'declare'?
By init, do you mean declare?

So you want 3x 'var accesskey = ...' ?
(In reply to comment #10)
>(Note it is indeed disabled on OS X; We do allow to turn on accesskeys though.)
Ah, so we need to provide them in case they get turned on, great.
>please remove single line brackets.
Bah, aaronlev introduced them into xpfe's dialog.xml :-( Mind you, he's not the
only culprit, you manged to check in both sorts for bug 284776 :-P

(In reply to comment #11)
>So you want 3x 'var accesskey = ...' ?
The point is that there will only be one in the end because of the #ifdefs.
Attachment #196644 - Attachment is obsolete: true
Attachment #197682 - Flags: review+
(In reply to comment #12)
> (In reply to comment #11)
> >So you want 3x 'var accesskey = ...' ?
> The point is that there will only be one in the end because of the #ifdefs.

Yes, I realize that, I was just confused about 'decale'.

I'm not sure who could sr ?
Mano, please double-check the patch and check it in for Vidar.
mozilla/toolkit/content/widgets/wizard.xml  1.24
mozilla/toolkit/locales/en-US/chrome/global/wizard.properties 1.5
mozilla/xpfe/global/resources/content/bindings/wizard.xml 1.25
mozilla/xpfe/global/resources/locale/en-US/mac/wizard.properties 1.4
mozilla/xpfe/global/resources/locale/en-US/unix/wizard.properties 1.2
mozilla/xpfe/global/resources/locale/en-US/win/wizard.properties 1.2
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed] (attachment 196643)
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 196643 [details] [diff] [review]
xpfe version 0.3

a=me for SM1.0b on SM only part of code, 2nd needed one - Horray!
SeaMonkey-only portion of patch checked in to the 1.8 branch.
Whiteboard: fixed-seamonkey1.0
Would someone take a look at bug 320736.  Possibly a regression from this bug.
Attachment #197682 - Flags: branch-1.8.1?(mconnor)
Attachment #197682 - Flags: branch-1.8.1?(mconnor) → branch-1.8.1+
mozilla/toolkit/content/widgets/wizard.xml;                    1.23.2.2;
mozilla/toolkit/locales/en-US/chrome/global/wizard.properties;  1.4.8.1;
Whiteboard: fixed-seamonkey1.0
Target Milestone: mozilla1.9alpha → mozilla1.8.1
You need to log in before you can comment on or make changes to this bug.