Checkbox not clickable when parent element has tabindex

VERIFIED FIXED in Firefox 38

Status

()

defect
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: kemenaran, Assigned: arai)

Tracking

({regression})

38 Branch
mozilla39
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox37 unaffected, firefox38+ verified, firefox39+ verified)

Details

(Whiteboard: [bugday-20150610], )

Attachments

(2 attachments)

Reporter

Description

4 years ago
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).
Reporter

Updated

4 years ago
Status: UNCONFIRMED → NEW
Component: Untriaged → CSS Parsing and Computation
Ever confirmed: true
Product: Firefox → Core
[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
Component: CSS Parsing and Computation → General
Flags: needinfo?(catalin.varga)
Tracking for 38 and 39 since this looks like a recent web compatibility regression.
Assignee

Comment 4

4 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)
Removing the needinfo as the regression window was provided by Alice in comment 3
Flags: needinfo?(catalin.varga)
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

4 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

4 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 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

4 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.
https://hg.mozilla.org/mozilla-central/rev/39a861ac2bb6
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Tooru, could you fill the uplift request to 38? thanks
Flags: needinfo?(arai.unmht)
Assignee

Comment 14

4 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?
Attachment #8581285 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee: nobody → arai.unmht
QA Whiteboard: [good first verify]

Comment 16

4 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

4 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)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.