[Webkit-unassigned] [Bug 83446] Broken handling of pseudo-elements in selectors API

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Apr 27 04:26:16 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=83446


Arpita Bahuguna <arpitabahuguna at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |arpitabahuguna at gmail.com,
                   |                            |eric at webkit.org,
                   |                            |mitz at webkit.org,
                   |                            |vijayan.bits at gmail.com




--- Comment #2 from Arpita Bahuguna <arpitabahuguna at gmail.com>  2012-04-27 04:26:16 PST ---
The issue:
The SelectorChecker::checkSelector() method is common for both setting and retrieving the rules. For differentiating between the same m_isCollectingRulesOnly (member of SelectorChecker) is used. This boolean is set when querying for the rules i.e. whenever APIs such as getMatchedCSSRules() or querySelectorAll() are called. Thus in the checkOneSelector() function we are able to skip the code that tries to set flags etc. at the time of setting the renderStyle for renderers.

Now, when querying for pseudo-element selectors via the querySelectorAll() API, checkOneSelector() method returns a true irrespective of whether the element really has the specified selector applied on it or not.
While handling the pseudo-element in checkOneSelector() we do an initial check [if (!context.elementStyle && !m_isCollectingRulesOnly)] but since we set m_isCollectingRuleOnly to true (SelectorQuery constructor) we actually fall-through thereby returning a true.
The check for !m_isCollectingRulesOnly is required for the case when the call comes from CSSStyleSelector::pseudoStyleRulesForElement() [the getMatchedCSSRules() API], in which case we don't want it to return false immediately.

The possible approaches:
1. Instead of maintaining the m_isCollectingRulesOnly flag we can rather have an enumeration that associates the checkSelector() call with its originating context. For instance, instead of setting m_isCollectingRulesOnly whenever a query rules call is made (i.e. from either SelectorQuery or CSSStyleSelector::pseudoStyleRulesForElement()), we can instead set the corresponding enum based on which we can either skip checking for pseudo-elements (querySelector) and return a false from checkOneSelector() method or continue (CSSStyleSelector::pseudoStyleRulesForElement() - getMatchedCSSRules()).

2. In the templated SelectorDataList::execute() method itself (called from SelectorDataList::queryAll()/queryFirst() alone) we can skip making the selectorChecker.checkSelector() call altogether if the selector queried for is a pseudo-element selector.

3. Overloading the SelectorChecker::checkSelector() method and passing an extra boolean parameter (default - false) which is set only when called from the SelectorDataList::execute() method.

Would really appreciate if someone could review the aforementioned scenarios and recommend the best approach for fixing this issue.
In my opinion, having an enum that associates the checkSelector() call with its originating context would be more appropriate.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list