RESOLVED FIXED 140878
Fix CSS Selector's tag name matching when mixing HTML and XML
https://bugs.webkit.org/show_bug.cgi?id=140878
Summary Fix CSS Selector's tag name matching when mixing HTML and XML
Benjamin Poulain
Reported 2015-01-25 22:34:48 PST
Fix CSS Selector's tag name matching when mixing HTML and XML
Attachments
Patch (338.09 KB, patch)
2015-01-25 23:01 PST, Benjamin Poulain
darin: review+
Benjamin Poulain
Comment 1 2015-01-25 23:01:06 PST
Darin Adler
Comment 2 2015-01-25 23:37:14 PST
Comment on attachment 245329 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245329&action=review > Source/WebCore/css/CSSSelector.h:330 > unsigned m_hasRareData : 1; > + unsigned m_hasNameWithCase : 1; It’s a little inconsistent how the data is stored in a single data union, but the flags for what’s in that union are fully independent. > Source/WebCore/css/CSSSelector.h:380 > + NameWithCase *m_nameWithCase; Wrong position for the "*" here, it should be by the type. > Source/WebCore/css/RuleSet.h:223 > + if (isHTMLName) > + return m_tagLowercaseLocalNameRules.get(key); > + return m_tagLocalNameRules.get(key); Given these are both the same type it might be better to share the same call to get and just chose the map with the boolean. Will the compiler’s common subexpression code notice these two giant inlined functions are identical? > Source/WebCore/css/SelectorChecker.cpp:547 > } > > + > +static inline bool tagMatches(const Element& element, const CSSSelector& simpleSelector) Extra blank line here. > Source/WebCore/css/SelectorFilter.cpp:129 > + (*hash++) = tagLowercaseLocalName.impl()->existingHash() * TagNameSalt; The parentheses around (*hash++) on this line and others make this code look strange to me. Could we omit those? > Source/WebCore/css/SelectorFilter.cpp:132 > break; > + } > default: This is not where we normally put the braces for switch statement cases. You should look at other code or the style guide for guidance. > Source/WebCore/cssjit/SelectorCompiler.cpp:188 > + const CSSSelector* tagNameSelector; I suggest putting the default { nullptr } here instead of in the constructor. Would work well for other data members too. > Source/WebCore/cssjit/SelectorCompiler.cpp:231 > + const CSSSelector* tagNameSelector; I suggest putting the default { nullptr } here instead of in the constructor. > Source/WebCore/cssjit/SelectorCompiler.cpp:232 > bool inverted; And here too, and then get rid of the constructor. > Source/WebCore/cssjit/SelectorCompiler.cpp:1374 > + if (!lhs || !rhs) > return TagNameEquality::MaybeEqual; This behavior of null is strange.
Benjamin Poulain
Comment 3 2015-01-26 12:40:21 PST
Benjamin Poulain
Comment 4 2015-01-27 01:37:49 PST
*** Bug 83438 has been marked as a duplicate of this bug. ***
Benjamin Poulain
Comment 5 2015-01-27 01:38:34 PST
*** Bug 79444 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.