Commit 67ffbbbb authored by jan.koester's avatar jan.koester
Browse files

Don't let unverifiable ancestor selectors force display:none/visibility:hidden



collectApproximateMatches treats an ancestor compound that reduced to
nothing (attribute selector or unsupported pseudo-class stripped away,
see stripAttributeSelectors) as vacuously satisfied, same as before.
But a rule that only matched because of that is now barred from
contributing display:none/visibility:hidden specifically, since a wrong
guess there deletes the element and its subtree from the import rather
than just mis-styling it. Confirmed on a real page-builder site where
every decorative spacer div was being hidden because its visibility
rule's ancestor condition was "[data-mode=horizontal]", a condition
this matcher has no way to verify.

Co-Authored-By: default avatarClaude Sonnet 5 <noreply@anthropic.com>
parent 8efdbf57
Loading
Loading
Loading
Loading
+36 −4
Original line number Diff line number Diff line
@@ -195,15 +195,30 @@ namespace {
    // to nothing, see splitCombinatorChain's callers) is vacuously satisfied
    // without consuming an ancestor slot -- an unverifiable requirement
    // shouldn't by itself invalidate an otherwise plausible match, same
    // "not proven to not match" philosophy as the rest of this file.
    // "not proven to not match" philosophy as the rest of this file. If @p
    // usedUnverifiableAncestor is non-null, it's set to true whenever this
    // happens, so a caller merging declarations into a props map (see
    // collectApproximateMatches) can single out display:none/
    // visibility:hidden from such a match -- unlike most properties, a wrong
    // guess there doesn't just mis-style an element, it makes the element
    // and its whole subtree disappear from the import (confirmed on a real
    // page-builder site: literally every spacer div was hidden because its
    // visibility rule's ancestor condition was an attribute selector like
    // "[data-mode=horizontal]", which this matcher can't verify and so
    // reduces to an empty, vacuously-satisfied compound -- see
    // stripAttributeSelectors).
    bool ancestorChainSatisfies(const std::vector<CompoundParts> &ancestorCompounds,
                                 const std::vector<libhtmlpp::AncestorFrame> &ancestors)
                                 const std::vector<libhtmlpp::AncestorFrame> &ancestors,
                                 bool *usedUnverifiableAncestor = nullptr)
    {
        size_t idx = 0;
        for (const auto &compound : ancestorCompounds) {
            bool specifiedSomething =
                !compound.tag.empty() || !compound.classes.empty() || !compound.id.empty();
            if (!specifiedSomething) continue;
            if (!specifiedSomething) {
                if (usedUnverifiableAncestor) *usedUnverifiableAncestor = true;
                continue;
            }

            bool found = false;
            while (idx < ancestors.size()) {
@@ -947,12 +962,14 @@ void libhtmlpp::CSSStyleSheet::collectApproximateMatches(
            compound.id = branch.id;
            if (!compoundMatches(compound, branch.hadCombinator, tagLower, classes, id)) continue;

            bool usedUnverifiableAncestor = false;
            if (ancestors && !branch.ancestorCompounds.empty()) {
                std::vector<CompoundParts> ancestorCompounds;
                for (const auto &ac : branch.ancestorCompounds) {
                    ancestorCompounds.push_back({ac.tag, ac.classes, ac.id});
                }
                if (!ancestorChainSatisfies(ancestorCompounds, *ancestors)) continue;
                if (!ancestorChainSatisfies(ancestorCompounds, *ancestors, &usedUnverifiableAncestor))
                    continue;
            }

            if (branch.isAtRule) {
@@ -984,6 +1001,21 @@ void libhtmlpp::CSSStyleSheet::collectApproximateMatches(
                    if (inlineKeys.count(key) && !isImportant) continue;
                    if (importantKeys.count(key) && !isImportant) continue;

                    // A match that only went through because an ancestor
                    // condition we can't verify (see
                    // ancestorChainSatisfies/usedUnverifiableAncestor) was
                    // treated as vacuously satisfied is exactly the kind of
                    // "maybe wrong" guess this file otherwise tolerates --
                    // except here: display:none/visibility:hidden don't
                    // just mis-style the element, they remove it and its
                    // whole subtree from the import. Drop just these two
                    // destructive values from an unverifiable match; every
                    // other property from the same rule still applies.
                    if (usedUnverifiableAncestor &&
                        ((key == "display" && value == "none") ||
                         (key == "visibility" && value == "hidden")))
                        continue;

                    props[key] = value;
                    if (isImportant) importantKeys.insert(key);
                }
+10 −1
Original line number Diff line number Diff line
@@ -208,7 +208,16 @@ namespace libhtmlpp {
         *  as described there -- nullptr (the default) preserves this
         *  function's original behavior (leading compounds of a combinator
         *  selector are never verified), a non-null chain makes leading
         *  compounds required to be found in it, in order. */
         *  compounds required to be found in it, in order. Exception: if a
         *  match only went through because one of its ancestor compounds
         *  was unverifiable (an attribute selector or unsupported pseudo-
         *  class stripped down to nothing -- see approximateSelectorMatch's
         *  own doc comment), that rule's display:none/visibility:hidden
         *  values are dropped rather than folded into @p props, since an
         *  ordinary wrong guess mis-styles an element but a wrong guess on
         *  those two properties makes it (and its subtree) disappear
         *  outright; every other property from the same rule is unaffected
         *  and still applies normally. */
        void collectApproximateMatches(const std::string &tag,
                                        const std::string &cssClass,
                                        const std::string &id,