Closed Bug 81481 (Legend-Accesskey) Opened 23 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
389 bytes, text/html
Details
testcase taken from W3C HTML 4.01 section 17.10 (legend)
2.11 KB, text/html
Details
Legends have tabindex here
2.16 KB, text/html
Details
Patch rev. 3
5.66 KB, patch
aaronlev
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
Patch rev. 4
6.67 KB, 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) — — 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) — — 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) — — 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 patch
          
          
        Patch rev. 3
        — 
          — 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
        — 
          — 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: