Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define what tabIndex returns when the tabindex attribute is not set #4754

Merged
merged 6 commits into from
Jul 9, 2019
Merged

Define what tabIndex returns when the tabindex attribute is not set #4754

merged 6 commits into from
Jul 9, 2019

Conversation

rakina
Copy link
Member

@rakina rakina commented Jul 5, 2019

See #1786 (comment).

This is consistent with behaviors in Chrome, Safari, Firefox - see table in #4464 (comment)

(though Chrome is failing in shadow host with delegatesFocus case - filed a bug at https://bugs.chromium.org/p/chromium/issues/detail?id=982195)

WPT: web-platform-tests/wpt#17657


/interaction.html ( diff )
/interactive-elements.html ( diff )

source Outdated
@@ -73709,8 +73709,10 @@ END:VCARD</pre>

<p>The <dfn><code data-x="dom-tabIndex">tabIndex</code></dfn> IDL attribute must
<span>reflect</span> the value of the <code data-x="attr-tabindex">tabindex</code> content
attribute. Its default value is 0 for elements that are focusable and &#x2212;1 for elements that
are not focusable.</p>
attribute. If the <code data-x="attr-tabindex">tabindex</code> content attribute is not set, then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default value is a term of art that we need to keep using. (The definition of reflect says "...if the attribute is absent, the default value must be returned instead".) I will push a fixup.

@domenic domenic added interop Implementations are not interoperable with each other normative change topic: focus and removed interop Implementations are not interoperable with each other labels Jul 8, 2019
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, very nice.

Given that this matches all browser engines, we should be able to merge this at will. I will give it a day or so if folks want to comment more.

Let's be sure to note that this closes #1786 and #4464 when merging, and also reference #4607.

@domenic
Copy link
Member

domenic commented Jul 8, 2019

Also, can you link to the Chromium bug for the failing test case?

@rniwa
Copy link
Collaborator

rniwa commented Jul 9, 2019

Filed a WebKit bug.

@rakina
Copy link
Member Author

rakina commented Jul 9, 2019

source Outdated
are not focusable.</p>
attribute. Its default value is 0 if the element is an <code>input</code>, <code>button</code>,
<code>a</code>, <code>textarea</code>, <code>select</code>, <code>iframe</code>, or
<code>area</code> element, and is -1 otherwise.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a −, not a -. (Probably best to keep using the entity reference.)

One other thing I suspect is that in Firefox it might be 0 for link elements as well, given that it treats them as links generally.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. https://boom-bath.glitch.me/tabindex.html updated and shows link is -1 in all browsers, so it doesn't need to be on this list. But this prompted me to do a bit more investigation and I found option and optgroup.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Er, actually my test had a typo. Those are still -1. But, summary is 0. Sigh.

@domenic domenic merged commit 15cf23a into whatwg:master Jul 9, 2019
@domenic
Copy link
Member

domenic commented Jul 9, 2019

Sigh, I merged this too soon :(. By code inspection of Gecko I discovered more special elements to test, namely embed, object, and SVG a.

I've added those to the web platform tests, and will create a follow-up PR for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants