Closed
Bug 344850
Opened 18 years ago
Closed 15 years ago
Tabbing from <radio> or <textbox> with tabindex fails
Categories
(Core :: DOM: UI Events & Focus Handling, defect, P3)
Core
DOM: UI Events & Focus Handling
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
People
(Reporter: stephend, Assigned: enndeakin)
References
(Blocks 1 open bug)
Details
(4 keywords, Whiteboard: [patchlove] [See comment 24, & forget Xpfe file])
Attachments
(6 files)
1.04 KB,
application/vnd.mozilla.xul+xml
|
Details | |
506 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
482 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
363 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
11.14 KB,
patch
|
neil
:
review+
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
4.64 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
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•18 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.
Comment 2•18 years ago
|
||
[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•18 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
Updated•18 years ago
|
Keywords: regression
Whiteboard: [SergeG: I'll try to find the regression date...]
Comment 4•18 years ago
|
||
[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...
Comment 5•18 years ago
|
||
[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.
Comment 6•18 years ago
|
||
**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...]
Comment 7•18 years ago
|
||
(In reply to comment #6) > *File and the radio group are +/- ignored (I did look precisely.) Ah: 'I did _not_ look precisely'.
Updated•18 years ago
|
Assignee: composer → mats.palmgren
Comment 8•18 years ago
|
||
Comment 9•18 years ago
|
||
Comment 10•18 years ago
|
||
Comment 11•18 years ago
|
||
Updated•18 years ago
|
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
Comment 12•18 years ago
|
||
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)
Comment 13•18 years ago
|
||
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 14•18 years ago
|
||
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•18 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•18 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 17•18 years ago
|
||
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 18•18 years ago
|
||
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•18 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•18 years ago
|
Attachment #231513 -
Flags: review?(aaronleventhal)
Comment 20•18 years ago
|
||
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)
Comment 21•18 years ago
|
||
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 22•18 years ago
|
||
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•18 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•18 years ago
|
Whiteboard: needs landing
Updated•18 years ago
|
Whiteboard: needs landing → [checkin needed]
Reporter | ||
Updated•18 years ago
|
Severity: normal → major
Reporter | ||
Updated•18 years ago
|
Flags: blocking1.9?
Reporter | ||
Updated•18 years ago
|
Whiteboard: [checkin needed]
Comment 25•18 years ago
|
||
Mats, could you post a new patch with comment 24 suggestions ?
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Updated•17 years ago
|
Priority: -- → P1
Updated•17 years ago
|
Whiteboard: [has-patch]
Updated•17 years ago
|
Whiteboard: [has-patch] → [has-patch] [See comment 24, & forget Xpfe file]
Comment 26•17 years ago
|
||
Is this patch still relevant? Issue still there? Stephen or Mats can you catch us up?
Priority: P1 → P3
Reporter | ||
Comment 27•17 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.
Comment 28•17 years ago
|
||
Mats, ping?
Updated•16 years ago
|
Flags: tracking1.9+ → wanted-next+
Comment 29•16 years ago
|
||
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•16 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•16 years ago
|
||
Zach, any chance you could look at this?
Reporter | ||
Updated•15 years ago
|
Flags: blocking1.9.2?
Comment 32•15 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•15 years ago
|
Assignee: matspal → nobody
Updated•15 years ago
|
Keywords: helpwanted
QA Contact: keyboard.navigation
Whiteboard: [has-patch] [See comment 24, & forget Xpfe file] → [patchlove] [See comment 24, & forget Xpfe file]
Comment 33•15 years ago
|
||
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•15 years ago
|
||
Updated•15 years ago
|
Attachment #391349 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 35•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/5cb576a2923a
Assignee | ||
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Flags: blocking1.9.2? → blocking1.9.2+
Updated•5 years ago
|
Component: Keyboard: Navigation → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•