Resolve direction correctly when dir attribute is not in a defined state

Source/core/html/HTMLElement.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2004-2008, 2013, 2014 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2011 Motorola Mobility. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 193 matching lines @@
     } else if (name == draggableAttr) {
         if (equalIgnoringCase(value, "true")) {
             addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitUserDrag, CSSValueElement);
             addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitUserSelect, CSSValueNone);
         } else if (equalIgnoringCase(value, "false"))
             addPropertyToPresentationAttributeStyle(style, CSSPropertyWebkitUserDrag, CSSValueNone);
     } else if (name == dirAttr) {
         if (equalIgnoringCase(value, "auto"))
             addPropertyToPresentationAttributeStyle(style, CSSPropertyUnicodeBidi, unicodeBidiAttributeForDirAuto(this));
         else {
-            if (isValidDirAttribute(value))
+            if (isValidDirAttribute(value)) {
                 addPropertyToPresentationAttributeStyle(style, CSSPropertyDirection, value);
-            else
-                addPropertyToPresentationAttributeStyle(style, CSSPropertyDirection, "ltr");

[Sunil Ratnu, 2014/11/21 12:21:08]

Hi tkent, We need to keep this else check here because removing it breaks
fast/dom/Attr/direction-attribute-set-and-cleared.html test. The parent lookup
fails here and keeps the direction "rtl" even after clearing it to "" as
mentioned in the test. So, we need to make sure that we consider parent
element's directionality and resolve accordingly. The mentioned test fails no
more now.

[tkent, 2014/11/25 01:27:24]

I don't think parent lookup is a right solution.
How about
    style->removeProperty(CSSPropertyDirection);
?

[Sunil Ratnu, 2014/11/25 05:43:25]

The above approach does not work here. The above mentioned test still fails.
Since this is the only place where we are checking for invalid dirAttr or
clearing of dirAttr and finally adding the direction to CSSPropertyDirection, we
need parent lookup so sa to resolve the direction and that seems reasonable.

[tkent, 2014/11/25 05:56:09]

Please investigate why it didn't work, and find another solution without parent
lookup.  Adding parent lookup is not acceptable.  Style resolution already
contains parent lookup, and adding DOM parent lookup will introduce
inconsistency and confusion.

+            } else {
+                const AtomicString& dirValue = determineDirection() == RTL ? "rtl" : "ltr";
+                addPropertyToPresentationAttributeStyle(style, CSSPropertyDirection, dirValue);
+            }
             if (!hasTagName(bdiTag) && !hasTagName(bdoTag) && !hasTagName(outputTag))
                 addPropertyToPresentationAttributeStyle(style, CSSPropertyUnicodeBidi, CSSValueEmbed);
         }
     } else if (name.matches(XMLNames::langAttr))
         mapLanguageAttributeToLocale(value, style);
     else if (name == langAttr) {
         // xml:lang has a higher priority than lang.
         if (!fastHasAttribute(XMLNames::langAttr))
             mapLanguageAttributeToLocale(value, style);
     } else
@@ skipping 480 matching lines @@
 {
     if (!(selfOrAncestorHasDirAutoAttribute() && hasDirectionAuto())) {
         isAuto = false;
         return LTR;
     }
 
     isAuto = true;
     return directionality();
 }
 
+// FIXME: Implement directionality for input type='tel'.
+// Spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#the-directionality.
+TextDirection HTMLElement::determineDirection() const

[Sunil Ratnu, 2014/11/21 12:21:08]

Instead of moving it here, we can also define a new function
effectiveDirAttributeValue (as suggested by you in case of input type='tel') but
that also has to use a lot of same code here. Hence, I moved it from
HTMLTextFormControlElement.cpp in order to reduce the duplicated code.

+{
+    for (const HTMLElement* element = this; element; element = Traversal<HTMLElement>::firstAncestor(*element)) {
+        const AtomicString& dirAttributeValue = element->fastGetAttribute(dirAttr);
+        if (dirAttributeValue.isNull())
+            continue;
+
+        if (equalIgnoringCase(dirAttributeValue, "rtl"))
+            return RTL;
+
+        if (equalIgnoringCase(dirAttributeValue, "ltr"))
+            return LTR;
+
+        if (equalIgnoringCase(dirAttributeValue, "auto")) {
+            bool isAuto;
+            return element->directionalityIfhasDirAutoAttribute(isAuto);
+        }
+    }
+
+    return LTR;
+}
+
 TextDirection HTMLElement::directionality(Node** strongDirectionalityTextNode) const
 {
     if (isHTMLInputElement(*this)) {
         HTMLInputElement* inputElement = toHTMLInputElement(const_cast<HTMLElement*>(this));
         bool hasStrongDirectionality;
         TextDirection textDirection = determineDirectionality(inputElement->value(), hasStrongDirectionality);
         if (strongDirectionalityTextNode)
             *strongDirectionalityTextNode = hasStrongDirectionality ? inputElement : 0;
         return textDirection;
     }
@@ skipping 305 matching lines @@
 #ifndef NDEBUG
 
 // For use in the debugger
 void dumpInnerHTML(blink::HTMLElement*);
 
 void dumpInnerHTML(blink::HTMLElement* element)
 {
     printf("%s\n", element->innerHTML().ascii().data());
 }
 #endif