Closed Bug 417760 Opened 12 years ago Closed 12 years ago

cannot focus() img elements with tabindex="-1"

Categories

(Core :: Disability Access APIs, defect, P2, major)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: simon.bates, Assigned: aaronlev)

References

(Blocks 1 open bug)

Details

(Whiteboard: Simple patch. Needed for ARIA when img is used for child widgets)

Attachments

(2 files)

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
Attachment #303652 - Flags: superreview?(mats.palmgren)
Attachment #303652 - Flags: review?(mats.palmgren)
Sorry, I thought
-1 means not focusable
0, focusable but not navigable
>0 defines navigation order

Isn't it true?
Blocks: aria
Severity: normal → major
Flags: blocking1.9?
Whiteboard: Simple patch. Seeking r+. Needed for ARIA when img is used for child widgets
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
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?
Attachment #303652 - Flags: superreview?(mats.palmgren)
Attachment #303652 - Flags: superreview+
Attachment #303652 - Flags: review?(mats.palmgren)
Attachment #303652 - Flags: review+
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
Flags: in-testsuite?
Attachment #303652 - Flags: approval1.9?
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?
Depends on: 424304
(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.
Filed bug 425087 to followup on the remaining issues from comment 6.
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.