Changeset 67506 in webkit

Timestamp:

Sep 14, 2010 3:53:02 PM (14 years ago)

Author:

abarth@webkit.org

Message:

2010-09-14 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

incorrect tabindex parsing
​https://bugs.webkit.org/show_bug.cgi?id=21076

Updated our integer parsing for tabindex to use the algorithm from the
HTML5 spec.

Test: fast/parser/tabindex-parsing-2.html

• html/HTMLElement.cpp: (WebCore::HTMLElement::parseMappedAttribute):
• html/parser/HTMLParserIdioms.cpp: (WebCore::parseHTMLInteger):
• html/parser/HTMLParserIdioms.h:

2010-09-14 Adam Barth <​abarth@webkit.org>

Reviewed by Eric Seidel.

incorrect tabindex parsing
​https://bugs.webkit.org/show_bug.cgi?id=21076

• fast/parser/tabindex-parsing-2-expected.txt: Added.
• fast/parser/tabindex-parsing-2.html: Added.
  • Test a bunch of corner cases in tabindex parsing.
• fast/parser/tabindex-parsing-expected.txt:
• fast/parser/tabindex-parsing.html:
  • Update expected result. This case is actually the one that the original reporter was complaining about.

Location:

trunk

Files:

• JavaScriptCore/wtf/text/WTFString.h (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/parser/tabindex-parsing-2-expected.txt (added)
• LayoutTests/fast/parser/tabindex-parsing-2.html (added)
• LayoutTests/fast/parser/tabindex-parsing-expected.txt (modified) (1 diff)
• LayoutTests/fast/parser/tabindex-parsing.html (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/html/HTMLElement.cpp (modified) (2 diffs)
• WebCore/html/parser/HTMLParserIdioms.cpp (modified) (1 diff)
• WebCore/html/parser/HTMLParserIdioms.h (modified) (1 diff)
• WebCore/wml/WMLElement.cpp (modified) (1 diff)

trunk/JavaScriptCore/wtf/text/WTFString.h

@@ -460 +460 @@
 using WTF::appendNumber;
 using WTF::charactersAreAllASCII;
+using WTF::charactersToIntStrict;
+using WTF::charactersToUIntStrict;
+using WTF::charactersToInt64Strict;
+using WTF::charactersToUInt64Strict;
+using WTF::charactersToIntPtrStrict;
+using WTF::charactersToInt;
+using WTF::charactersToUInt;
+using WTF::charactersToInt64;
+using WTF::charactersToUInt64;
+using WTF::charactersToIntPtr;
 using WTF::charactersToDouble;
 using WTF::charactersToFloat;
-using WTF::charactersToInt;
 using WTF::equal;
 using WTF::equalIgnoringCase;

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-09-14  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        incorrect tabindex parsing
+        https://bugs.webkit.org/show_bug.cgi?id=21076
+
+        * fast/parser/tabindex-parsing-2-expected.txt: Added.
+        * fast/parser/tabindex-parsing-2.html: Added.
+            - Test a bunch of corner cases in tabindex parsing.
+        * fast/parser/tabindex-parsing-expected.txt:
+        * fast/parser/tabindex-parsing.html:
+            - Update expected result.  This case is actually the one that the
+              original reporter was complaining about.
+
 2010-09-14  Jian Li  <jianli@chromium.org>
 

trunk/LayoutTests/fast/parser/tabindex-parsing-expected.txt

@@ -10 +10 @@
 .tabIndex=7 getAttribute('tabindex')='007'
 
-This element shouldn't be focusable PASSED
-.tabIndex=-1 getAttribute('tabindex')='1px'
+This element should be focusable PASSED
+.tabIndex=1 getAttribute('tabindex')='1px'
 
 This element should be focusable PASSED

trunk/LayoutTests/fast/parser/tabindex-parsing.html

@@ -60 +60 @@
 <span id="sp3">Click to test manually</span>
 </p>
-<p tabindex="1px" onfocus="p4Focused=true" onclick="test(p4Focused,false,4)">This element shouldn't be focusable <br>
+<p tabindex="1px" onfocus="p4Focused=true" onclick="test(p4Focused,true,4)">This element should be focusable <br>
 <span id="sp4">Click to test manually</span>
 </p>

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-09-14  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Eric Seidel.
+
+        incorrect tabindex parsing
+        https://bugs.webkit.org/show_bug.cgi?id=21076
+
+        Updated our integer parsing for tabindex to use the algorithm from the
+        HTML5 spec.
+
+        Test: fast/parser/tabindex-parsing-2.html
+
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::parseMappedAttribute):
+        * html/parser/HTMLParserIdioms.cpp:
+        (WebCore::parseHTMLInteger):
+        * html/parser/HTMLParserIdioms.h:
+
 2010-09-14  Brent Fulgham  <bfulgham@webkit.org>
 

trunk/WebCore/html/HTMLElement.cpp

@@ -40 +40 @@
 #include "HTMLFormElement.h"
 #include "HTMLNames.h"
+#include "HTMLParserIdioms.h"
 #include "RenderWordBreak.h"
 #include "ScriptEventListener.h"
@@ -143 +144 @@
     } else if (attr->name() == tabindexAttr) {
         indexstring = getAttribute(tabindexAttr);
-        if (indexstring.length()) {
-            bool parsedOK;
-            int tabindex = indexstring.toIntStrict(&parsedOK);
-            if (parsedOK)
-                // Clamp tabindex to the range of 'short' to match Firefox's behavior.
-                setTabIndexExplicitly(max(static_cast<int>(std::numeric_limits<short>::min()), min(tabindex, static_cast<int>(std::numeric_limits<short>::max()))));
+        int tabindex = 0;
+        if (parseHTMLInteger(indexstring, tabindex)) {
+            // Clamp tabindex to the range of 'short' to match Firefox's behavior.
+            setTabIndexExplicitly(max(static_cast<int>(std::numeric_limits<short>::min()), min(tabindex, static_cast<int>(std::numeric_limits<short>::max()))));
         }
     } else if (attr->name() == langAttr) {

trunk/WebCore/html/parser/HTMLParserIdioms.cpp

@@ -92 +92 @@
 }
 
+// http://www.whatwg.org/specs/web-apps/current-work/#rules-for-parsing-integers
+bool parseHTMLInteger(const String& input, int& value)
+{
+    // Step 1
+    // Step 2
+    const UChar* position = input.characters();
+    const UChar* end = position + input.length();
+
+    // Step 3
+    int sign = 1;
+
+    // Step 4
+    while (position < end) {
+        if (!isHTMLSpace(*position))
+            break;
+        ++position;
+    }
+
+    // Step 5
+    if (position == end)
+        return false;
+    ASSERT(position < end);
+
+    // Step 6
+    if (*position == '-') {
+        sign = -1;
+        ++position;
+    } else if (*position == '+')
+        ++position;
+    if (position == end)
+        return false;
+    ASSERT(position < end);
+
+    // Step 7
+    if (!isASCIIDigit(*position))
+        return false;
+
+    // Step 8
+    Vector<UChar, 16> digits;
+    while (position < end) {
+        if (!isASCIIDigit(*position))
+            break;
+        digits.append(*position++);
+    }
+
+    // Step 9
+    value = sign * charactersToIntStrict(digits.data(), digits.size());
+    return true;
 }
+
+}

trunk/WebCore/html/parser/HTMLParserIdioms.h

@@ -46 +46 @@
 bool parseToDoubleForNumberType(const String&, double*);
 
+// http://www.whatwg.org/specs/web-apps/current-work/#rules-for-parsing-integers
+bool parseHTMLInteger(const String&, int&);
+
 // Inline implementations of some of the functions declared above.
 

trunk/WebCore/wml/WMLElement.cpp

@@ -73 +73 @@
     } else if (attr->name() == HTMLNames::tabindexAttr) {
         String indexstring = attr->value();
-        if (indexstring.length()) {
-            bool parsedOK;
-            int tabindex = indexstring.toIntStrict(&parsedOK);
-            if (parsedOK)
-                // Clamp tabindex to the range of 'short' to match Firefox's behavior.
-                setTabIndexExplicitly(max(static_cast<int>(std::numeric_limits<short>::min()), min(tabindex, static_cast<int>(std::numeric_limits<short>::max()))));
+        int tabindex = 0;
+        if (parseHTMLInteger(tabindex)) {
+            // Clamp tabindex to the range of 'short' to match Firefox's behavior.
+            setTabIndexExplicitly(max(static_cast<int>(std::numeric_limits<short>::min()), min(tabindex, static_cast<int>(std::numeric_limits<short>::max()))));
         }
     }