Stop clamping tabIndex to short range

third_party/WebKit/Source/core/dom/Element.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Peter Kelly (pmk@post.com)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2007 David Smith (catfish.man@gmail.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012, 2013 Apple Inc.
  * All rights reserved.
  *           (C) 2007 Eric Seidel (eric@webkit.org)
  *
@@ skipping 199 matching lines @@
   if (!hasRareData())
     return;
   elementRareData()->clearElementFlag(mask);
 }
 
 void Element::clearTabIndexExplicitlyIfNeeded() {
   if (hasRareData())
     elementRareData()->clearTabIndexExplicitly();
 }
 
-void Element::setTabIndexExplicitly(short tabIndex) {
+void Element::setTabIndexExplicitly(int tabIndex) {
   ensureElementRareData().setTabIndexExplicitly(tabIndex);
 }
 
 void Element::setTabIndex(int value) {
   setIntegralAttribute(tabindexAttr, value);
 }
 
-short Element::tabIndex() const {
+int Element::tabIndex() const {
   return hasRareData() ? elementRareData()->tabIndex() : 0;
 }
 
 bool Element::layoutObjectIsFocusable() const {
   // Elements in canvas fallback content are not rendered, but they are allowed
   // to be focusable as long as their canvas is displayed and visible.
   if (isInCanvasSubtree()) {
     const HTMLCanvasElement* canvas =
         Traversal<HTMLCanvasElement>::firstAncestorOrSelf(*this);
     DCHECK(canvas);
@@ skipping 2187 matching lines @@
   if (name == tabindexAttr) {
     int tabindex = 0;
     if (value.isEmpty()) {
       clearTabIndexExplicitlyIfNeeded();
       if (adjustedFocusedElementInTreeScope() == this) {
         // We might want to call blur(), but it's dangerous to dispatch
         // events here.
         document().setNeedsFocusedElementCheck();
       }
     } else if (parseHTMLInteger(value, tabindex)) {
-      // Clamp tabindex to the range of 'short' to match Firefox's behavior.
-      setTabIndexExplicitly(
-          max(static_cast<int>(std::numeric_limits<short>::min()),
-              std::min(tabindex,
-                       static_cast<int>(std::numeric_limits<short>::max()))));
+      setTabIndexExplicitly(tabindex);

[kochi, 2016/10/18 02:37:24]

Could you add a comment here for the behavior when the parsed value is
out of int ragne?

As far as I read parseHTMLInteger(), if the value is out of int range,
it returns false and thus tabindex will be 0 (set on line 2427).

But in that case, setTabIndexExplicitly(tabindex) will not be called?

If so, it can make some difference in the following case:

1. <div tabindex="3"></div>
    => div.tabIndex = 3
2. div.setAttribute("tabindex", "12039810238102983");
3. the div becomes <div tabindex="12039810238102983"><div> but
   div.tabIndex = 3 or 0?

If you have a test case to catch this, it is nice.

[rwlbuis, 2016/10/19 00:43:43]

Done.
That is correct.
I added a test for this. Turns out it ends up being 3 since the setAttribute
tries to set to an out of range value.

     }
   } else if (name == XMLNames::langAttr) {
     pseudoStateChanged(CSSSelector::PseudoLang);
   }
 }
 
 bool Element::parseAttributeName(QualifiedName& out,
                                  const AtomicString& namespaceURI,
                                  const AtomicString& qualifiedName,
                                  ExceptionState& exceptionState) {
@@ skipping 1631 matching lines @@
 }
 
 DEFINE_TRACE_WRAPPERS(Element) {
   if (hasRareData()) {
     visitor->traceWrappers(elementRareData());
   }
   ContainerNode::traceWrappers(visitor);
 }
 
 }  // namespace blink