Add support for case-insensitive attribute value selectors

Source/core/css/SelectorChecker.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2004-2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Nicholas Shanks (webkit@nickshanks.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007, 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
@@ skipping 480 matching lines @@
     const QualifiedName& selectorAttr = selector.attribute();
     ASSERT(selectorAttr.localName() != starAtom); // Should not be possible from the CSS grammar.
 
     // Synchronize the attribute in case it is lazy-computed.
     // Currently all lazy properties have a null namespace, so only pass localName().
     element.synchronizeAttribute(selectorAttr.localName());
 
     if (!element.hasAttributesWithoutUpdate())
         return false;
 
-    const AtomicString& selectorValue =  selector.value();
+    const AtomicString& selectorValue = selector.value();
+    bool caseInsensitive = selector.attributeFlags() & CSSSelector::CaseInsensitive;
 
     AttributeIteratorAccessor attributes = element.attributesIterator();
     AttributeConstIterator end = attributes.end();
     for (AttributeConstIterator it = attributes.begin(); it != end; ++it) {
         const Attribute& attributeItem = **it;
 
         if (!attributeItem.matches(selectorAttr))
             continue;
 
-        if (attributeValueMatches(attributeItem, match, selectorValue, true))
+        if (attributeValueMatches(attributeItem, match, selectorValue, !caseInsensitive))
             return true;
 
+        if (caseInsensitive)
+            continue;
+
         // Case sensitivity for attribute matching is looser than hasAttribute or
         // Element::shouldIgnoreAttributeCase() for now. Unclear if that's correct.
         bool caseSensitive = !element.document().isHTMLDocument() || HTMLDocument::isCaseSensitiveAttribute(selectorAttr);

[Timothy Loh, 2014/07/22 08:51:02]

This is super confusing (we have a caseSensitive and a caseInsensitive variable
in the same scope)! Can we rename things and tweak comments to aid comprehension
here?

[fs, 2014/07/22 11:06:17]

I suppose we could make the new (currently |caseInsensitive|)
|selectorCaseInsensitive| or somesuch. Maybe |matchAttrCaseSensitive(ly)| for
the current |caseSensitive|? Naming is hard though, so I'll gladly take
suggestions...

What kind of comment tweaks were you envisioning?

[Timothy Loh, 2014/07/22 11:50:26]

If I'm reading crbug.com/327060 right, the existing |caseSensitive| branch is
legacy behaviour that we want to remove and this patch will allow us to tweak
our UA stylesheets to not hit this case. So perhaps keep |caseInsensitive| as
is, but change |caseSensitive| into |legacyCaseInsensitive|.

I would write a comment like "Legacy behavior is to treat values for some
attributes as case insensitive regardless of whether the case insensitive flag
is set" but feel free to write what makes the most sense to you; the existing
bug link here is probably the most useful part.

[fs, 2014/07/22 14:09:04]

Thank you for elaborating. I've updated to something like the above.

[Timothy Loh, 2014/07/23 01:46:50]

I think |legacyCaseInsensitive| (instead of |legacyCaseSensitive|) makes this
slightly easier to understand

 
         // If case-insensitive, re-check, and count if result differs.
         // See http://code.google.com/p/chromium/issues/detail?id=327060
         if (!caseSensitive && attributeValueMatches(attributeItem, match, selectorValue, false)) {
             UseCounter::count(element.document(), UseCounter::CaseInsensitiveAttrSelectorMatch);
             return true;
         }
     }
 
     return false;
@@ skipping 622 matching lines @@
     return element.focused() && isFrameFocused(element);
 }
 
 template
 SelectorChecker::Match SelectorChecker::match(const SelectorCheckingContext&, const DOMSiblingTraversalStrategy&, MatchResult*) const;
 
 template
 SelectorChecker::Match SelectorChecker::match(const SelectorCheckingContext&, const ShadowDOMSiblingTraversalStrategy&, MatchResult*) const;
 
 }