Closed
Bug 1141455
Opened 10 years ago
Closed 10 years ago
Checkbox not clickable when parent element has tabindex
Categories
(Core :: General, defect)
Tracking
()
VERIFIED
FIXED
mozilla39
Tracking | Status | |
---|---|---|
firefox37 | --- | unaffected |
firefox38 | + | verified |
firefox39 | + | verified |
People
(Reporter: kemenaran, Assigned: arai)
References
()
Details
(Keywords: regression, Whiteboard: [bugday-20150610])
Attachments
(2 files)
16.26 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
16.29 KB,
patch
|
arai
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150309004203
Steps to reproduce:
1. Have an HTML structure of label -> div with tabindex -> checkbox-input
2. Click inside the div with tabindex
Actual results:
The checkbox is not selected.
Expected results:
The checkbox should be selected.
The actual behavior is:
* clicking on the label area toggles the checkbox (correctly)
* clicking on the checkbox itself toggles the checkbox (correctly)
* BUT clicking on the div with `tabindex` doesn't toggle the checkbox (incorrectly)
Here is a JSBin that demonstrates the issue: http://jsbin.com/budisehuyo/3/
This issue is present in Firefox 38a2 and 39a1 (but not before; it's a regression). It appears on www.capitainetrain.com (see 1135374).
Updated•10 years ago
|
Status: UNCONFIRMED → NEW
Component: Untriaged → CSS Parsing and Computation
Ever confirmed: true
Product: Firefox → Core
Comment 1•10 years ago
|
||
[Tracking Requested - why for this release]: Web compat regression
Uh... This has nothing to do with CSS. There's no CSS in the testcase at all.
Presumably the issue is that the click is focusing the scrollable area itself.
Blocks: 1135374
tracking-firefox38:
--- → ?
tracking-firefox39:
--- → ?
Component: CSS Parsing and Computation → General
Flags: needinfo?(catalin.varga)
Keywords: regression,
regressionwindow-wanted
Comment 2•10 years ago
|
||
Tracking for 38 and 39 since this looks like a recent web compatibility regression.
status-firefox37:
--- → unaffected
status-firefox38:
--- → affected
status-firefox39:
--- → affected
Comment 3•10 years ago
|
||
Assignee | ||
Comment 4•10 years ago
|
||
I think current behavior matches to what the spec says.
https://html.spec.whatwg.org/multipage/forms.html#the-label-element:the-label-element-10
> The activation behaviour of a label element for events targeted at interactive
> content descendants of a label element, and any descendants of those
> interactive content descendants, must be to do nothing.
https://html.spec.whatwg.org/multipage/dom.html#interactive-content-2
> Interactive content is content that is specifically intended for user
> interaction.
> The tabindex attribute can also make any element into interactive content.
tabindex makes span element an interactive content, and label's activation behaviour must be to do nothing, instead of clicking on checkbox, for the event on the span element.
So, this should be a spec issue.
To make it work same as before, removing `HasAttr(kNameSpaceID_None, nsGkAtoms::tabindex)` condition from nsGenericHTMLElement::IsInteractiveHTMLContent will solve.
https://hg.mozilla.org/mozilla-central/file/0190a1d17294/dom/html/nsGenericHTMLElement.cpp#l1801
> bool
> nsGenericHTMLElement::IsInteractiveHTMLContent() const
> {
> return IsAnyOfHTMLElements(nsGkAtoms::details, nsGkAtoms::embed,
> nsGkAtoms::keygen) ||
> HasAttr(kNameSpaceID_None, nsGkAtoms::tabindex);
> }
smaug, what's the appropriate solution for this case?
Flags: needinfo?(arai.unmht) → needinfo?(bugs)
Comment 5•10 years ago
|
||
Removing the needinfo as the regression window was provided by Alice in comment 3
Flags: needinfo?(catalin.varga)
Comment 6•10 years ago
|
||
What do other browsers do here? Sounds like we need to follow their behavior and get the spec fixed.
(Please file a spec bug.)
Perhaps removing the tabindex check is the right thing here. But then, having tabindex makes element interactive... clearly the spec needs clarification.
For now, perhaps we could add a param to IsInteractiveHTMLContent to tell whether or not check
tabindex attribute.
Flags: needinfo?(bugs)
Assignee | ||
Comment 7•10 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #6)
> What do other browsers do here? Sounds like we need to follow their behavior
> and get the spec fixed.
Tested on following, and clicking on span with tabindex turns on/off the checkbox on all of them, so now only Firefox 38.0a2 and 39.0a1 behave differently.
Windows 7
IE 11.09600.17633
Mac OS X 10.9.5
Google Chrome 41.0.2272.89 (64-bit)
Google Chrome Canary 43.0.2331.3 canary (64-bit)
Safari 7.1.3 (9537.85.12.18)
WebKit r181478
> (Please file a spec bug.)
Filed here: https://www.w3.org/Bugs/Public/show_bug.cgi?id=28212
> For now, perhaps we could add a param to IsInteractiveHTMLContent to tell
> whether or not check
> tabindex attribute.
I'll make a patch.
Assignee | ||
Comment 8•10 years ago
|
||
Added ignoreTabindex parameter to IsInteractiveHTMLContent, and pass true from InInteractiveHTMLContent in HTMLLabelElement.cpp.
Almost green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fced6d072b4
Attachment #8578379 -
Flags: review?(bugs)
Comment 9•10 years ago
|
||
Comment on attachment 8578379 [details] [diff] [review]
Do not treat an element with tabindex as an interactive content in label.
s/ignoreTabindex/aIgnoreTabindex/
I realized the current handling of tabindex doesn't even follow the spec, since
any element with tabindex should make it interactive.
So, HTMLObjectElement::IsInteractiveHTMLContent for example should call its parent class to see if tabindex is used.
But that fix can be done in a different bug, since it doesn't affect the fix.
Attachment #8578379 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 10•10 years ago
|
||
Thank you for reviewing :D
(In reply to Olli Pettay [:smaug] from comment #9)
> Comment on attachment 8578379 [details] [diff] [review]
> Do not treat an element with tabindex as an interactive content in label.
>
> s/ignoreTabindex/aIgnoreTabindex/
fixed locally :)
I'll push it to m-i after tree open.
> I realized the current handling of tabindex doesn't even follow the spec,
> since
> any element with tabindex should make it interactive.
> So, HTMLObjectElement::IsInteractiveHTMLContent for example should call its
> parent class to see if tabindex is used.
> But that fix can be done in a different bug, since it doesn't affect the fix.
Oops, thanks for pointing it out, filed as bug 1144322.
Assignee | ||
Comment 11•10 years ago
|
||
Comment 12•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment 13•10 years ago
|
||
Tooru, could you fill the uplift request to 38? thanks
Flags: needinfo?(arai.unmht)
Assignee | ||
Comment 14•10 years ago
|
||
Thank you for the reminder :)
Backported to mozilla-aurora. no major changes.
Approval Request Comment
[Feature/regressing bug #]: bug 229925
[User impact if declined]: Label element doesn't work as expected, and becomes non-clickable in some case.
[Describe test coverage new/current, TreeHerder]: both in test suite (dom/html/test/forms/test_interactive_content_in_label.html)
[Risks and why]: Low. it disables a part of changes in bug 229925. so it just becomes previous behavior.
[String/UUID change made/needed]: no
Flags: needinfo?(arai.unmht)
Attachment #8581285 -
Flags: review+
Attachment #8581285 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8581285 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 15•10 years ago
|
||
Thank you again!
https://hg.mozilla.org/releases/mozilla-aurora/rev/325daf43b8f1
Updated•10 years ago
|
Assignee: nobody → arai.unmht
Updated•10 years ago
|
QA Whiteboard: [good first verify]
Comment 16•9 years ago
|
||
Successfully reproduce the bug of Aurora 38a2 (build ID: 20150310004228) nigthly 39a1 (build ID: 20150310030235) .
Fixed works for me on Firefox 39.0 Linux 64 bit (build ID : 20150604162752 , User Agent Mozilla/5.0 (X11; Linux x86_64; rv:39.0) Gecko/20100101 Firefox/39.0)
QA Whiteboard: [good first verify] → [good first verify][bugday-20150610]
Whiteboard: [bugday-20150610]
Comment 17•9 years ago
|
||
Fixed works for me on Firefox 38.0.5 Windows (build ID : 20150525141253 , Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0)
You need to log in
before you can comment on or make changes to this bug.
Description
•