Closed Bug 260527 Opened 20 years ago Closed 20 years ago

Prefs -> Navigator radio buttons not underlining accesskeys in labels

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.8beta2

People

(Reporter: aaronlev, Assigned: aaronlev)

References

Details

(Keywords: access)

Attachments

(2 files, 3 obsolete files)

The navigator pref window radio buttons (Home page, Blank page, Last page) are
actually in a deck which changes depending on the combo box choice. The
accesskey is applied late to the radio button.

There are several problems with the code:
1. The prefwindow sets accesskey dynamically via attribute instead of js property
2. Setting accesskey dynamically doesn't update the appearance of <radio
label="foo">
Attached patch Still has one problem (obsolete) — Splinter Review
Unfortunately, the accesskeys disappear when you down arrow and then up arrow
back to "Navigator" in the category pane.

Neil, any idea why?
Attached patch Four-fold fix (obsolete) — Splinter Review
1) Change h->m for Ho_m_e page which conflicts with _H_elp 
2) Use accesskey property instead of attribute for dynamic changes
3) Prefer accesskey attribute be stored on control, not label which is often an
anonymous part of the control
4) Call setPageAccessKeys() each time panel is shown, not just first time
(otherwise accesskeys disappear when you go away and back to that panel)
Attachment #159484 - Attachment is obsolete: true
Attachment #159490 - Flags: superreview?(neil.parkwaycc.co.uk)
URL: ]
Priority: -- → P1
Attachment #159490 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #159781 - Flags: superreview?(neil.parkwaycc.co.uk)
Blocks: 260605
Attachment #159781 - Attachment is obsolete: true
Attachment #159781 - Flags: superreview?(neil.parkwaycc.co.uk)
Comment on attachment 159860 [details] [diff] [review]
Switch order of accesskey attribute setting

All I did was switch the order of some lines of code in general.xml
Attachment #159860 - Flags: superreview?(neil.parkwaycc.co.uk)
Comment on attachment 159860 [details] [diff] [review]
Switch order of accesskey attribute setting

>@@ -247,12 +247,10 @@ function init()
>   } catch(e) {
>   }
> 
>   gData.navigatorData = navigatorData;
> 
>-  setPageAccessKeys(document.getElementById("behaviourDeck").firstChild);
>-
>   prefWindow.registerOKCallbackFunc(doOnOk);
> }
> 
> function Startup()
> {
>@@ -265,10 +263,12 @@ function Startup()
>     setHomePageValue(homePages[0]);
>   else
>     setHomePageValue(navigatorData.groupIsSet);
> 
>   updateHomePageButtons();
>+
>+  setPageAccessKeys(document.getElementById("behaviourDeck").firstChild);
> }
Not sure why you moved this as Startup() calls init().

>+            var accessKey;
I think you should initialize it rather than leaving it undefined if there's no
labelled control element.
Attachment #159860 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
I moved setPageAccessKeys to Startup() because Init() checks to see if
initialization has already happened. If we don't move it, then down arrow and up
arrow again and the accesskeys dissappear.
Attachment #159860 - Flags: review?(mconnor)
Comment on attachment 159860 [details] [diff] [review]
Switch order of accesskey attribute setting

Ok.  With neil's comments about not leaving accessKey as undefined, r=me.
Attachment #159860 - Flags: review?(mconnor) → review+
Checking in toolkit/content/widgets/text.xml;
/cvsroot/mozilla/toolkit/content/widgets/text.xml,v  <--  text.xml
new revision: 1.7; previous revision: 1.6
done
Checking in toolkit/content/widgets/general.xml;
/cvsroot/mozilla/toolkit/content/widgets/general.xml,v  <--  general.xml
new revision: 1.6; previous revision: 1.5
done
Checking in xpfe/components/prefwindow/resources/content/pref-navigator.js;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/content/pref-navigator.js,v
 <--  pref-navigator.js
new revision: 1.16; previous revision: 1.15
done
Checking in xpfe/components/prefwindow/resources/locale/en-US/pref-navigator.dtd;
/cvsroot/mozilla/xpfe/components/prefwindow/resources/locale/en-US/pref-navigator.dtd,v
 <--  pref-navigator.dtd
new revision: 1.25; previous revision: 1.24
done
Checking in xpfe/global/resources/content/bindings/general.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/general.xml,v  <-- 
general.xml
new revision: 1.32; previous revision: 1.31
done
Checking in xpfe/global/resources/content/bindings/text.xml;
/cvsroot/mozilla/xpfe/global/resources/content/bindings/text.xml,v  <--  text.xml
new revision: 1.13; previous revision: 1.12
done
Checking in content/xul/content/src/nsXULElement.cpp;
/cvsroot/mozilla/content/xul/content/src/nsXULElement.cpp,v  <--  nsXULElement.cpp
new revision: 1.546; previous revision: 1.545
done
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Keywords: aviary-landing
Relanding relevant parts of patch following aviary branch landing.
Keywords: aviary-landing
Keeping
{{
neil.parkwaycc.co.uk: superreview+
mconnor: review+ 
}}
from
{{
(In reply to comment #6)
> (From update of attachment 159860 [details] [diff] [review] [edit])
> >+		var accessKey;
> I think you should initialize it rather than leaving it undefined if there's
no
> labelled control element.

(In reply to comment #8)
> (From update of attachment 159860 [details] [diff] [review] [edit])
> Ok.  With neil's comments about not leaving accessKey as undefined, r=me.
}}

Aaron (or Neil), Can you check it in on my behalf ? Thanks.
Attachment #176799 - Flags: superreview+
Attachment #176799 - Flags: review+
URL: ]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
OS: Windows XP → All
Hardware: PC → All
Comment on attachment 176799 [details] [diff] [review]
(Ev1) <text.xml> forgotten suggested fix
[Checked in: Comment 12]


Check in: { 2005-03-10 06:29	aaronleventhal%moonset.net	mozilla/
toolkit/ content/ widgets/ text.xml    1.12 }
Attachment #176799 - Attachment description: (Ev1) <text.xml> forgotten suggested fix → (Ev1) <text.xml> forgotten suggested fix [Checked in: Comment 12]
Attachment #176799 - Attachment is obsolete: true
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.8beta2
Attachment #176799 - Attachment is obsolete: false
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: