Closed Bug 417760 Opened 12 years ago Closed 12 years ago
cannot focus() img elements with tabindex="-1"
To reproduce: * open the attached test file focus-img-tabindex-minus-one.html * click on the button 'focus img tabindex="-1"' Expected: * focus to go to the img with tabindex="-1" Actual: * focus does not move Tested with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008021504 Minefield/3.0b4pre
Sorry, I thought -1 means not focusable 0, focusable but not navigable >0 defines navigation order Isn't it true?
No, that's not right. See this table: http://developer.mozilla.org/en/docs/Key-navigable_custom_DHTML_widgets#The_solution:_changes_to_standard_behavior_of_tabindex
Severity: normal → major
Whiteboard: Simple patch. Seeking r+. Needed for ARIA when img is used for child widgets
Mats, this is marked blocking. How about some review love?
Comment on attachment 303652 [details] [diff] [review] Simple patch -- check if tabindex attr present This looks good. r+sr=mats Please add a mochitest as well, calling focus() on a few different elements and verify it has focus (or not) using 'activeElement' for example. At least something simple that covers this bug. That said, there are still a couple of issues in this code, although these can probably wait to post-1.9: 1. we should return false if !IsInDoc(), not just for image map: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/content/src/nsHTMLImageElement.cpp&rev=1.224&root=/cvsroot&mark=442#436 and a better test is probably what nsGenericHTMLElement::IsFocusable does: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/content/src/nsGenericHTMLElement.cpp&rev=1.756&root=/cvsroot&mark=3119-3127,3133#3116 2. this block should be removed?: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/html/content/src/nsHTMLImageElement.cpp&rev=1.224&root=/cvsroot&mark=461-464#436 <input type="image"> is not handled by this method, if that was the assumption We should refactor these methods (post-1.9) such that a subclass IsFocusable() can re-use common code from a superclass... Please file a followup bug?
Whiteboard: Simple patch. Seeking r+. Needed for ARIA when img is used for child widgets → Simple patch. Needed for ARIA when img is used for child widgets
Comment on attachment 303652 [details] [diff] [review] Simple patch -- check if tabindex attr present You are blocking - approval not needed. Land away
Attachment #303652 - Flags: approval1.9? → approval1.9+
checked in /cvsroot/mozilla/content/html/content/src/nsHTMLImageElement.cpp,v <-- nsHTMLImageElement.cpp new revision: 1.225; previous revision: 1.224 done
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
So... the review included a request for a test. I assume that's being worked on, still?
Marco, since Aaron is away several days then could you care about mochitest for this?
(In reply to comment #9) > So... the review included a request for a test. I assume that's being worked > on, still? I've created follow-up bug 424304 for this Mochitest. I'll work on it there.
verified fixed using the testcase and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9pre) Gecko/2008042705 Minefield/3.0pre ID:2008042705
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.