New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 654860 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Stop clamping tabIndex to short range

Reported by rob.b...@samsung.com, Oct 11 2016

Issue description

We should clamp tabIndex to integer instead of short like other implementations:
https://html.spec.whatwg.org/multipage/interaction.html#dom-tabindex
 
Owner: rob.b...@samsung.com
Status: Assigned (was: Untriaged)

Comment 2 by kochi@chromium.org, Oct 17 2016

Cc: kochi@chromium.org
Components: Blink>Focus
IIRC the spec used to clamp on 16-bit short range (up to 32767).

I haven't researched much yet, but quick search found some links:

http://stackoverflow.com/questions/24924851/when-does-tabindex-xxxx-break
> Browsers actually enforce the limit, though in incompatible ways.
> Chrome and Firefox interpret larger values as 32767 or (for very large
> numbers) 0, IE as negative numbers (except that 32768 is taken as 0).

https://msdn.microsoft.com/en-us/library/ms534654(v=vs.85).aspx
> For Internet Explorer 5.01 or above, the attribute may be set to any
> value in the valid range of -32767 to 32767

I think changing the limit will not have big compat issue, as such large
tabindex are rarely used, or not useful at all.  I think this change is
not urgent (so P3 seems appropriate).

Could you clarify the implications for compat issue, e.g. other browsers' behavior, when the spec was updated, etc?

> Could you clarify the implications for compat issue, e.g. other browsers' behavior, when the spec was updated, etc?

The most relevant link I found is this thread:
https://lists.w3.org/Archives/Public/public-whatwg-archive/2014Jul/0153.html

I don't know when the html spec changed exactly, but the 32767 limit is removed since a long time:
https://www.w3.org/TR/2010/WD-html5-20101019/editing.html#sequential-focus-navigation-and-the-tabindex-attribute

From the thread we can see that mozilla implemented this in 2014:
https://bugzilla.mozilla.org/show_bug.cgi?id=996095

Webkit implemented this earlier this year:
https://bugs.webkit.org/show_bug.cgi?id=155159

Edge has not implemented this behavior.

Comment 4 by kochi@chromium.org, Oct 18 2016

@rob.buis thanks for the research!
As Mozilla and WebKit already does this, it seems safe to ship this in
Chrome.

As this is a web-exposed change, it makes sense to send a note
(in Intent to Implement and Ship format) to blink-dev. Could you work on this?
You seem to have all information to fill in the format:)
Summary: Stop clamping tabIndex to short range (was: Clamp tabIndex to int range)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4

commit dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4
Author: rob.buis <rob.buis@samsung.com>
Date: Sat Nov 05 00:21:59 2016

Stop clamping tabIndex to short range

Stop clamping tabIndex to short range to match
the specification [1] and other implementations.

Setting this attribute with values in range [INT_MIN, INT_MAX]
will be accepted, otherwise rejected.

The test fast/events/max-tabindex-focus.html is taken
from WebKit and rewritten to use testharness.js.

Behavior matches Firefox and Safari.

BUG= 654860 

[1] https://html.spec.whatwg.org/multipage/interaction.html#dom-tabindex

Review-Url: https://codereview.chromium.org/2362223002
Cr-Commit-Position: refs/heads/master@{#430080}

[add] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/fast/dom/tabindex-behavior.html
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/fast/dom/tabindex-clamp-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/fast/dom/tabindex-clamp.html
[add] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/fast/events/max-tabindex-focus.html
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-embedded-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-forms-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-grouping-expected.txt
[delete] https://crrev.com/fb52b0f9af558002450baf25cacbc98feb9201e2/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-metadata-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-misc-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-obsolete-expected.txt
[delete] https://crrev.com/fb52b0f9af558002450baf25cacbc98feb9201e2/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-sections-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-tabular-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/LayoutTests/imported/wpt/html/dom/reflection-text-expected.txt
[delete] https://crrev.com/fb52b0f9af558002450baf25cacbc98feb9201e2/third_party/WebKit/LayoutTests/platform/win/fast/dom/tabindex-clamp-expected.txt
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/dom/Element.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/dom/ElementRareData.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/dom/ElementRareData.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/dom/Node.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLAnchorElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLAnchorElement.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLElement.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLFieldSetElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLFieldSetElement.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLFormControlElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLFormControlElement.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLOutputElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLOutputElement.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLSlotElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/html/HTMLSlotElement.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/page/FocusController.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/svg/SVGAElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/svg/SVGAElement.h
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/svg/SVGElement.cpp
[modify] https://crrev.com/dcfa87e4ecf3f9673f642f9292f9a69e50ef6ba4/third_party/WebKit/Source/core/svg/SVGElement.h

Status: Fixed (was: Assigned)
Fixed by r430080.

Comment 8 by kochi@chromium.org, Nov 8 2016

Thanks!!

Confirmed that you already filed the Firefox bug, which is also awesome!
https://bugzilla.mozilla.org/show_bug.cgi?id=1315238

Components: Blink>HTML>Focus
Components: -Blink>Focus

Sign in to add a comment