bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Tabbing from <radio> or <textbox> with tabindex fails

RESOLVED FIXED

Status

()

Core
Keyboard: Navigation
P3
major
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: stephend, Assigned: Neil Deakin)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
access, helpwanted, regression, testcase
Points:
---
Bug Flags:
wanted-next +
blocking1.9.2 +
blocking1.9 -

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(Whiteboard: [patchlove] [See comment 24, & forget Xpfe file])

Attachments

(6 attachments)

(Reporter)

Description

12 years ago
Build ID: 2006-07-14-23

In Composer, I can't tab from the "Alternate Text" radio button into its adjacent textfield.
(Reporter)

Comment 1

12 years ago
Complete steps:

1. In Composer, click on Insert | Image...
2. Tab to "Alternate Text"

Expected Results:

Take me to the adjacent textfield

Actual Results:

Won't tab futher.
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b1) Gecko/20060717 SeaMonkey/1.1a] (nightly) (W98SE)

I confirm.

And Shift-Tab is not working from 'Tooltip' either.

Comment 3

12 years ago
SeaMonkey 1.0.0 has the bug, although Mozilla 1.7 does not, but that's still a big regression range, anyone care to start narrowing it down?
Keywords: access
Keywords: regression
Whiteboard: [SergeG: I'll try to find the regression date...]
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b1) Gecko/20060717 SeaMonkey/1.1a] (nightly) (W98SE)

I don't think it's related,
but (anyway) I get the following error each time the "Insert > Image..." dialog opens
[
Error: redeclaration of const MOZILLA_CONTENT_PACK
Source File: chrome://help/content/contextHelp.js
Line: 40
]

I don't seem to get the error with the other menu items...
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.13) Gecko/20060414] (release) (W98SE)


In this release, Tab and Shit-Tab work;
I was only kind of surprised that the (2 buttons) radio group comes "after" its associated textfield.
[Anyway, now I know what behaviour(s) to look for.]

***

[
Error: redeclaration of const MOZILLA_CONTENT_PACK
Source File: chrome://help/content/contextHelp.js
Line: 24
]
already there: that part is not a (recent) regression.

As a reminder (for now), 3 bugs have comments with this const name:
bug 162432, bug 271992, bug 260058.
**Summary:

I found 3 change/regression timeframes...
I searched and used the (+/- rarely) available Windows builds.
If further narrowing of the timeframes is needed, you'll have to do it with Linux builds.!.

**Details:

*Initially:
 *Working (like v1.7.13).

(...)
2004-04-10-11-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040410
2004-04-18-10-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040418
2004-06-25-09-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a2) Gecko/20040625
2004-06-26-09-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a2) Gecko/20040626

*After that build,
 *Much like the current behaviour, but it seems to need 2 Tab to leave the first textarea...

2004-07-12-15-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a2) Gecko/20040712

*After that build,
 *Tab "cycle": Location > Tooltip > Alt.Text (area) > 'Tab_Title'.
 *Shift-Tab "cycle": Tooltip < Alt.Text (area) < Cancel < Help < Location.
 *File and the radio group are +/- ignored (I did look precisely.)

2004-08-19-10-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a3) Gecko/20040819
2004-09-02-07-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a3) Gecko/20040902
2005-01-05-07-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a6) Gecko/20050105
2005-02-12-06-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050212
2005-03-04-07-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050304
2005-03-12-06-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050312
2005-03-14-06-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050314

*After that build,
 *Shift-Tab doesn't work in the (3) textareas.
 *Tab doesn't work on the radio group.

2005-03-15-07-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050315
2005-03-16-06-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050316
2005-03-20-06-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050320
2005-04-04-07-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050404
2005-04-05-07-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050405
[Before: http://archive; After: Ftp://ftp]
2005-07-05-07-trunk : Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050705 SeaMonkey/1.0a
(...)
Whiteboard: [SergeG: I'll try to find the regression date...]
(In reply to comment #6)
>  *File and the radio group are +/- ignored (I did look precisely.)

Ah: 'I did _not_ look precisely'.

Updated

12 years ago
Blocks: 83552
Assignee: composer → mats.palmgren
Created attachment 231508 [details]
Testcase #2b (as #2a but without tabindex)
Created attachment 231509 [details]
Testcase #3 (simple <textbox> only)
Component: Composer → Keyboard: Navigation
Keywords: testcase
OS: Windows XP → All
Product: Mozilla Application Suite → Core
Hardware: PC → All
Summary: Can't tab from the Alternate Text radio button into its textfield. → Tabbing from <radio> or <textbox> with tabindex fails
Version: unspecified → Trunk
Created attachment 231513 [details] [diff] [review]
Patch rev. 1

There are two separate bugs here.
1. when we have a radio control with a tabindex attribute (making it
   tabbable) we get into trouble because the radio always puts back
   focus on the radiogroup when it is focused. 
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/content/widgets/radio.xml&rev=1.23&root=/cvsroot&mark=433,437#433
   So, on the next TAB we start again at the radiogroup,
   finding the tabbable radio item that puts back focus on the group...
2. for anonymous content with a tabindex, like for example the html:input
   inside a textbox with a tabindex we have a simliar situation for
   Shift-TAB (the textbox puts the focus back on the html:input).

For the reported Composer dialog it would be enough to move the tabindex
attribute to the radiogroup (to fix 1), but I have included a fix that tries
to solve the problem when individual <radio> items are tabbable in case
someone wants to use that for some reason.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/editor/ui/dialogs/content/EdImageOverlay.xul&rev=1.10&root=/cvsroot&mark=72,91,96#71

With this patch I get the expected behaviour with both forward/backward
tabbing on the reported dialog, the testcases and a lot of other testing
I did on Linux. I also (briefly) tested Firefox on Windows and Camino on
MacOSX.

About the "control" property - it seems to me this have never worked so
we could just rename it to "radioGroup" (which is implemented) in the IDL
file instead, what do you think?
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/dom/public/idl/xul/nsIDOMXULSelectCntrlItemEl.idl&rev=1.3&root=/cvsroot&mark=55#43
Attachment #231513 - Flags: review?(bzbarsky)
I'm not sure I can sanely review the XUL changes; one of the Neils would be bette there...  I can try to do the ESM changes, but again it would be better if someone more familiar with the code reviewed; I'd be happy to sr, but I don't feel like I know this code well enough to do the r.
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

Neil, can you review the XUL changes please?
Bryner, can you review the ESM changes please?
Attachment #231513 - Flags: review?(neil)
Attachment #231513 - Flags: review?(bzbarsky)
Attachment #231513 - Flags: review?(bryner)

Comment 15

12 years ago
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

>+      <property name="control">
readonly="true" perhaps?
Attachment #231513 - Flags: review?(neil) → review+

Comment 16

12 years ago
(In reply to comment #12)
>About the "control" property - it seems to me this have never worked so
>we could just rename it to "radioGroup" (which is implemented) in the IDL
>file instead, what do you think?
I think that idea is worse than the reverse of renaming "radioGroup" to be "control", but your patch works too.
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

This code can be tricky and I don't have time to do a thorough review on it.  I'd suggest Aaron Leventhal.
Attachment #231513 - Flags: review?(bryner)
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

Aaron, can you review the nsEventStateManager changes please?
Attachment #231513 - Flags: review?(aaronleventhal)

Comment 19

12 years ago
Mats, I think you need an sr= more than a second review. I don't really want to or have time to touch this right now.

Updated

12 years ago
Attachment #231513 - Flags: review?(aaronleventhal)
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

I need review on the ESM changes and superreview on the whole patch.
Boris, I'm sorry my requests seems to land on your or roc's plate
every time, but I don't know anyone else who knows this code.
Attachment #231513 - Flags: superreview?(bzbarsky)
Attachment #231513 - Flags: review?(bzbarsky)
Mats, I'll try to figure this code out enough to review, but that will probably take time... :(  If it doesn't happen in the next week or so, it probably won't till after Sept 3, at least.
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

roc, if you're at all familiar with this code, could you review?  This is the bug I was telling you about the other day...
Attachment #231513 - Flags: superreview?(bzbarsky) → superreview?(roc)
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

I can't give this code good review, but generally I feel it's better to do what one can rather than block things indefinitely. Especially when the contributor is someone like Mats who I'm confident will chase down any regressions :-)
Attachment #231513 - Flags: superreview?(roc)
Attachment #231513 - Flags: superreview+
Attachment #231513 - Flags: review?(bzbarsky)
Attachment #231513 - Flags: review+

Comment 24

12 years ago
Comment on attachment 231513 [details] [diff] [review]
Patch rev. 1

>+    if (startRadioGroup &&
>+        NS_SUCCEEDED(startRadioGroup->GetSelectedItem(getter_AddRefs(radio))) &&
>+        radio) {
>+      nsIContent* start = nsnull;
>+      CallQueryInterface(radio, &start);
>+      if (start) {
>+        // Start traversing at the selected <radio> item.
>+        aStartFrame = start ?
>+          mPresContext->GetPresShell()->GetPrimaryFrameFor(start) : nsnull;
I just noticed... nsIContent is a refcounted interface, so you should do something like this:
if (startRadioGroup &&
    NS_SUCCEEDED(startRadioGroup->GetSelectedItem(getter_AddRefs(radio)))) {
  nsCOMPtr<nsIContent> start(do_QueryInterface(radio));
  if (start) {
    // Start traversing at the selected <radio> item.
    aStartFrame = mPresContext->GetPresShell()->GetPrimaryFrameFor(start);

>       else if ((aIgnoreTabIndex || mCurrentTabIndex == tabIndex) &&
>-          currentContent != aStartContent) {
>+               currentContent != aStartContent) {
Weird indentation change here.
(Reporter)

Updated

12 years ago
Whiteboard: needs landing

Updated

12 years ago
Whiteboard: needs landing → [checkin needed]
(Reporter)

Updated

12 years ago
Severity: normal → major
(Reporter)

Updated

12 years ago
Flags: blocking1.9?
(Reporter)

Updated

12 years ago
Whiteboard: [checkin needed]
Mats, could you post a new patch with comment 24 suggestions ?

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+

Updated

11 years ago
Priority: -- → P1

Updated

11 years ago
Whiteboard: [has-patch]
Whiteboard: [has-patch] → [has-patch] [See comment 24, & forget Xpfe file]

Comment 26

11 years ago
Is this patch still relevant?  Issue still there? Stephen or Mats can you catch us up?
Priority: P1 → P3
(Reporter)

Comment 27

11 years ago
(In reply to comment #26)
> Is this patch still relevant?  Issue still there? Stephen or Mats can you catch
> us up?

Schrep: I don't know about the patch, but the bug's still there; just waiting on Mats' follow-up patch/reply.
Mats, ping?
Flags: tracking1.9+ → wanted-next+
A regression in accessibility on the trunk with a testcase and an almost-completed patch isn't blocking?

Re-requesting blocking. There's no reason we shouldn't fix this.
Flags: blocking1.9?

Comment 30

11 years ago
> A regression in accessibility on the trunk with a testcase and an
> almost-completed patch isn't blocking?

Last patch was in 2006. Would love to get this in, of course.

Mats, ping?
Flags: blocking1.9? → blocking1.9-
(Reporter)

Comment 31

10 years ago
Zach, any chance you could look at this?
(Reporter)

Updated

10 years ago
Flags: blocking1.9.2?

Comment 32

9 years ago
I would imagine that there's no chance of getting this into 1.9.1 (FF 3.5), but it's an old bug that's still biting people, including in a web app we use daily.  Any chance of getting this bugfix rolling?
(Reporter)

Updated

9 years ago
Assignee: matspal → nobody
Keywords: helpwanted
QA Contact: keyboard.navigation
Whiteboard: [has-patch] [See comment 24, & forget Xpfe file] → [patchlove] [See comment 24, & forget Xpfe file]
This seems to have been fixed for 1.9.2 by the focus manager rewrite although I can't seem to tab backwards from any of the text fields for some reason.
(Assignee)

Comment 34

9 years ago
Created attachment 391349 [details] [diff] [review]
when tabbing backwards, skip nodes that would redirect their focus
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #391349 - Flags: review?(Olli.Pettay)
Attachment #391349 - Flags: review?(Olli.Pettay) → review+
(Assignee)

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Flags: blocking1.9.2? → blocking1.9.2+
(Assignee)

Comment 36

9 years ago
This is already fixed in 1.9.2.
status1.9.2: --- → beta1-fixed
You need to log in before you can comment on or make changes to this bug.