Closed Bug 81481 (Legend-Accesskey) Opened 24 years ago Closed 20 years ago

The accesskey attribute of the legend element is not working

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: bsharma, Assigned: MatsPalmgren_bugz)

References

(
URL
)

Details

(Keywords: access, html4, testcase)

Attachments

(5 files, 3 obsolete files)

The accesskey attribute of the legend element is not working
(deleted), text/html
Details
testcase taken from W3C HTML 4.01 section 17.10 (legend)
(deleted), text/html
Details
Legends have tabindex here
(deleted), text/html
Details
Patch rev. 3
(deleted), patch
aaronlev
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
Patch rev. 4
(deleted), patch
aaronlev
: review+
Details | Diff | Splinter Review
The accesskey attribute of the legend element is not working. This is the case with the Win XP platform also.
HTML Element component is deprecated, and should be removed from Bugzilla. Clayton is not the correct owner for these. Reassigning to layout.
Assignee: clayton → karnaze
Component: HTML Element → Layout
QA Contact: bsharma → petersen
Keywords: access
not a table specific bug, reassigning to core owner.
Assignee: karnaze → attinasi
reassinging to form control
Assignee: attinasi → rods
Component: Layout → HTML Form Controls
QA Contact: petersen → madhur
<html> <body> <p> The input fields have a accesskey attribute assigned. The first name field uses "f" and the last name field uses "l". <form action="" method="Post"> <label accesskey="f"><u>F</u>irst Name<input type="text" name="firstname"> </label> <label accesskey="l"><u>L</u>ast Name<input type="text" name="lastname"> </label> </form> </p> </body> </html>
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1
This is a major issue and is still happening with build 2002020103, adding keyword nsbeta1
Keywords: nsbeta1
Marking nsbeta1-. legends are a seldom used HTML element.
Keywords: nsbeta1 → nsbeta1-
Priority: -- → P1
Target Milestone: mozilla1.0.1 → Future
QA Contact: madhur → tpreston
Relevant references, quotes and links about this bug: http://www.w3.org/TR/html401/interact/forms.html#h-17.11.2 "Pressing an access key assigned to an element gives focus to the element. The action that occurs when an element receives focus depends on the element. (...) The invocation of access keys depends on the underlying system. For instance, on machines running MS Windows, one generally has to press the "alt" key in addition to the access key. On Apple systems, one generally has to press the "cmd" key in addition to the access key. The rendering of access keys depends on the user agent. We recommend that authors include the access key in label text or wherever the access key is to apply. User agents should render the value of an access key in such a way as to emphasize its role and to distinguish it from other characters (e.g., by underlining it)." http://www.w3.org/TR/html401/interact/forms.html#h-17.10 http://www.htmlhelp.com/reference/html40/forms/legend.html "LEGEND's ACCESSKEY attribute, used throughout the preceding example, specifies a single Unicode character as a shortcut key for giving focus to the LEGEND, allowing the user to quickly jump to a group of form controls. Entities (e.g. &eacute;) may be used as the ACCESSKEY value." http://www.algonet.se/~eva/ref/html/form/fieldset.html
This test case is a slightly modified version of the code found at http://www.w3.org/TR/html401/interact/forms.html#edef-LEGEND I only added the underlining to the accesskeys.
Blocks: robin's
*** Bug 226986 has been marked as a duplicate of this bug. ***
Keywords: testcase
Attached patch Partial fix (obsolete) (deleted) — — Splinter Review
This implements the attribute.. but there are focus issues
So... with that patch, we correctly scroll to the right <legend>. But it doesn't seem to get focused properly -- hitting tab after hitting the accesskey does not focus the first control in the fieldset. bryner, mats, do you know what's up?
Assignee: rods → nobody
Status: ASSIGNED → NEW
OS: Windows NT → All
Priority: P1 → --
QA Contact: tpreston → core.layout.form-controls
Hardware: PC → All
Target Milestone: Future → ---
I don't see that the scrolling works either with that patch. Anyway, I think moving focus to the legend element is a dead end that won't work. If you look at ESM::ShiftFocus/Internal and ESM::GetNextTabbableContent you will see that to get focus you need to support nsHTMLAtoms::tabindex on <legend> (at least if you don't want to mess with ESM to much). Also, I don't think this is desirable from a UI perspective either. IMO, typing the access-key should set focus on the first focusable element in the fieldset the legend belongs to. A way of implementing that, without changing ESM, could be to reset the current focus and then set the selection, then we can use the current code in ESM that in this case moves the focus to the next thing that is focusable after the selection. The problem is that I found that this is not working for <legend> at the moment (I have filed bug 236071 on this). I have worked around that bug for the moment and have something that seems to work. It's a bit hacky, but not to bad I think. Let me know if you're interested and I will clean it up and make a patch...
Yeah, that would be great, Mats. Thanks for looking into this!
Depends on: 236071
Attached patch Patch rev. 1 (obsolete) (deleted) — — Splinter Review
This patch only works correctly together with the patch for bug 236071.
Assignee: nobody → mats.palmgren
Attachment #142572 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #143133 - Flags: review?(bzbarsky)
Attached patch Patch rev. 2 (obsolete) (deleted) — — Splinter Review
Oops, forgot to remove #include "nsIFrameDebug.h"
Attachment #143133 - Attachment is obsolete: true
Attachment #143133 - Flags: review?(bzbarsky)
Attachment #143134 - Flags: review?(bzbarsky)
Comment on attachment 143134 [details] [diff] [review] Patch rev. 2 r=bzbarsky. Using a PRBool to pass around 0/1 instead of true/false is pretty wrong.... (not your fault, though).
Attachment #143134 - Flags: review?(bzbarsky) → review+
Keywords: nsbeta1- → html4
Alias: LegendAccesskey
Alias: LegendAccesskey → Legend-Accesskey
Comment on attachment 143134 [details] [diff] [review] Patch rev. 2 bit-rotted
Attachment #143134 - Attachment is obsolete: true
Attached file Legends have tabindex here (deleted) —
Attached patch Patch rev. 3 (deleted) — — Splinter Review
Updated to trunk with one addition; when the legend itself has been made focusable with a tabindex then focus it, not the next focusable element.
Attachment #169876 - Flags: superreview?(bzbarsky)
Attachment #169876 - Flags: review?(aaronleventhal)
Comment on attachment 169876 [details] [diff] [review] Patch rev. 3 Note: this patch needs the patch for bug 236071 to work as intended.
Comment on attachment 169876 [details] [diff] [review] Patch rev. 3 sr=bzbarsky. I really wish we could factor this accesskey stuff out into nsGenericHTMLFormElement or something... right now it's handled in a mix of content nodes and frames with code copy/pasted all over. Could you file a followup bug on this, cc me and Aaron Leventhal?
Attachment #169876 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 169876 [details] [diff] [review] Patch rev. 3 r+, conditional on using void advanceFocusIntoSubtree(in nsIDOMElement elt); from nsIDOMXULCommandDispatcher.idl: That's how XUL <dialog> advances the inital focus when a dialog opens. You can see it being used in dialog.xml. Seems cleaner to me. Agree on moving all the duplicate accesskey code into a base class. Most of the time we just need to check if an item IsFocsable() to know whether we should support accesskey on it. We can move the accessKey property into nsIDOMNSHTMLElement.idl, the same way we moved tabIndex there.
Attachment #169876 - Flags: review?(aaronleventhal) → review+
(In reply to comment #22) > r+, conditional on using > void advanceFocusIntoSubtree(in nsIDOMElement elt); I can't figure out how to get a nsIDOMXULDocument from here to be able to use GetCommandDispatcher(). So, I opted for using the FocusController directly (which other code does too). (AdvanceFocusIntoSubtree is just a wrapper for it anyway:) http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xul/document/src/nsXULCommandDispatcher.cpp&rev=1.81&root=/cvsroot&mark=199-205#197 Secondly, the problem is that FocusController::MoveFocus is buggy or at least does not do what I expected ;-) In the case the element is not focusable in itself it is disregarded as the starting point and the current tab index is used instead. Note that it does work in the case of newly created window since there is no current tab index and thus the first focusable element will receive focus, which is why AdvanceFocusIntoSubtree() seems to work. The bug is here: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/events/src/nsEventStateManager.cpp&rev=1.546&root=/cvsroot&mark=3184-3186#3142 IMO, there should be an "else ignoreTabIndex = PR_TRUE;" on that 'if'.
Attached patch Patch rev. 4 (deleted) — — Splinter Review
Something like this...
Attachment #170228 - Flags: review?(aaronleventhal)
BTW, I did check that this does not regress bug 254011.
Using XUL-specific methods in core HTML code seems like a bad idea, Aaron -- there are Gecko builds with XUL largely disabled...
For that matter, in an embedding build, do we have a chrome event handler on windows? That's what we use to get the focus controller....
Comment on attachment 170228 [details] [diff] [review] Patch rev. 4 You already had r= :) Anyway, I don't have time to look at MoveFocus, but since it works it looks good to me. I think you can just assert that domElement is not null, instead of testing it.
Attachment #170228 - Flags: review?(aaronleventhal) → review+
Filed bug 278623 on refactoring the accesskey stuff (comment 21 and 22). Checked in "Patch rev. 4" 2005-01-16 08:39 PDT -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Tested all testcases posted here under Windows XP Seamonkey build 2005-01-17-07, and they all work. Verified FIXED.
Status: RESOLVED → VERIFIED
One question before all this gets verified and closed. On this page, http://www.gtalbot.org/HTMLJavascriptCSS/AdvancedCSSButtons.html you have 2 <legend> with accesskey. If I do <alt>+G, the font-size select does not get focus but if I do <alt>+B, the Border Corner shape select gets focus. The problem is that the <object id="dlgHelperFonts"> (line 334 in the source code) gets in the way of the transfer of focus or the <object id="dlgHelperFonts"> "takes/captures" focus. I've tested this carefully. HTML 4 objects do not have onfocus attribute and <object> DOM 2 HTML does not have a focus() method; so <object> shouldn't be focusable. + // If the legend isn't focusable (no tabindex) we focus whatever is + // focusable following the legend instead, bug 81481. but <object> shouldn't be focusable. (bug 223518 ?) I can upload a reduced testcase if needed.
(In reply to comment #31) That's a separate problem - I filed bug 279378 on it.
*** Bug 281015 has been marked as a duplicate of this bug. ***
Depends on: 409604
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: