From d38f27840f62ecef36c569f0335be7667508f3d9 Mon Sep 17 00:00:00 2001 From: Marcel Greter Date: Thu, 2 Apr 2015 01:05:50 +0200 Subject: [PATCH] Fix `@extend` specificity problems Fixes https://github.com/sass/libsass/issues/592 Tried to implement specificity as per spec, but this turned out not to work like ruby sass. Once I tweaked the specificity constant of all types, it started to work, but I don't know why ruby sass does it that way! --- ast.hpp | 57 ++++++++++++++++++++++++++++----------------------- constants.cpp | 11 +++++++++- constants.hpp | 11 +++++++++- debugger.hpp | 27 ++++++++++++++++-------- extend.cpp | 4 ++-- 5 files changed, 71 insertions(+), 39 deletions(-) diff --git a/ast.hpp b/ast.hpp index af38d43d01..51e8c29e5c 100644 --- a/ast.hpp +++ b/ast.hpp @@ -1742,7 +1742,9 @@ namespace Sass { { } virtual ~Selector() = 0; // virtual Selector_Placeholder* find_placeholder(); - virtual int specificity() { return Constants::SPECIFICITY_BASE; } + virtual unsigned long long specificity() { + return Constants::Specificity_Universal; + }; }; inline Selector::~Selector() { } @@ -1787,10 +1789,10 @@ namespace Sass { Selector_Reference(ParserState pstate, Selector* r = 0) : Simple_Selector(pstate), selector_(r) { has_reference(true); } - virtual int specificity() + virtual unsigned long long specificity() { - if (selector()) return selector()->specificity(); - else return 0; + if (!selector()) return 0; + return selector()->specificity(); } ATTACH_OPERATIONS(); }; @@ -1817,10 +1819,11 @@ namespace Sass { Type_Selector(ParserState pstate, string n) : Simple_Selector(pstate), name_(n) { } - virtual int specificity() + virtual unsigned long long specificity() { - if (name() == "*") return 0; - else return 1; + // ToDo: What is the specificity of the star selector? + if (name() == "*") return Constants::Specificity_Type; + else return Constants::Specificity_Type; } virtual Compound_Selector* unify_with(Compound_Selector*, Context&); ATTACH_OPERATIONS(); @@ -1835,10 +1838,11 @@ namespace Sass { Selector_Qualifier(ParserState pstate, string n) : Simple_Selector(pstate), name_(n) { } - virtual int specificity() + virtual unsigned long long specificity() { - if (name()[0] == '#') return Constants::SPECIFICITY_BASE * Constants::SPECIFICITY_BASE; - else return Constants::SPECIFICITY_BASE; + if (name()[0] == '#') return Constants::Specificity_ID; + if (name()[0] == '.') return Constants::Specificity_Class; + else return Constants::Specificity_Type; } virtual Compound_Selector* unify_with(Compound_Selector*, Context&); ATTACH_OPERATIONS(); @@ -1855,6 +1859,10 @@ namespace Sass { Attribute_Selector(ParserState pstate, string n, string m, String* v) : Simple_Selector(pstate), name_(n), matcher_(m), value_(v) { } + virtual unsigned long long specificity() + { + return Constants::Specificity_Attr; + } ATTACH_OPERATIONS(); }; @@ -1868,17 +1876,6 @@ namespace Sass { Pseudo_Selector(ParserState pstate, string n, String* expr = 0) : Simple_Selector(pstate), name_(n), expression_(expr) { } - virtual int specificity() - { - // TODO: clean up the pseudo-element checking - if (name() == ":before" || name() == "::before" || - name() == ":after" || name() == "::after" || - name() == ":first-line" || name() == "::first-line" || - name() == ":first-letter" || name() == "::first-letter") - return 1; - else - return Constants::SPECIFICITY_BASE; - } virtual bool is_pseudo_element() { if (name() == ":before" || name() == "::before" || @@ -1888,10 +1885,18 @@ namespace Sass { return true; } else { - // If it's not a known pseudo-element, check whether it looks like one. This is similar to the type method on the Pseudo class in ruby sass. + // If it's not a known pseudo-element, check whether it looks like one. + // This is similar to the type method on the Pseudo class in ruby sass. return name().find("::") == 0; } } + virtual unsigned long long specificity() + { + /* needed to match ruby sass */ + if (name().find("::") == 0) + return Constants::Specificity_Type; + return Constants::Specificity_Pseudo; + } virtual Compound_Selector* unify_with(Compound_Selector*, Context&); ATTACH_OPERATIONS(); }; @@ -1946,7 +1951,7 @@ namespace Sass { return 0; } bool is_superselector_of(Compound_Selector* rhs); - virtual int specificity() + virtual unsigned long long specificity() { int sum = 0; for (size_t i = 0, L = length(); i < L; ++i) @@ -2007,7 +2012,7 @@ namespace Sass { // virtual Selector_Placeholder* find_placeholder(); Combinator clear_innermost(); void set_innermost(Complex_Selector*, Combinator); - virtual int specificity() const + virtual unsigned long long specificity() const { int sum = 0; if (head()) sum += head()->specificity(); @@ -2088,9 +2093,9 @@ namespace Sass { : Selector(pstate), Vectorized(s), wspace_(0) { } // virtual Selector_Placeholder* find_placeholder(); - virtual int specificity() + virtual unsigned long long specificity() { - int sum = 0; + unsigned long long sum = 0; for (size_t i = 0, L = length(); i < L; ++i) { sum += (*this)[i]->specificity(); } return sum; diff --git a/constants.cpp b/constants.cpp index 10692c72fb..9df0f004e3 100644 --- a/constants.cpp +++ b/constants.cpp @@ -2,7 +2,16 @@ namespace Sass { namespace Constants { - extern const int SPECIFICITY_BASE = 1000; + + // https://github.com/sass/libsass/issues/592 + // https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity + // https://github.com/sass/sass/issues/1495#issuecomment-61189114 + extern const unsigned long long Specificity_Universal = 1 << 0; + extern const unsigned long long Specificity_Type = 1 << 2; + extern const unsigned long long Specificity_Class = 1 << 3; + extern const unsigned long long Specificity_Attr = 1 << 3; + extern const unsigned long long Specificity_Pseudo = 1 << 3; + extern const unsigned long long Specificity_ID = 1 << 10; // sass keywords extern const char at_root_kwd[] = "@at-root"; diff --git a/constants.hpp b/constants.hpp index 4afe4fdf5e..7c170ff82d 100644 --- a/constants.hpp +++ b/constants.hpp @@ -3,7 +3,16 @@ namespace Sass { namespace Constants { - extern const int SPECIFICITY_BASE; + + // https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity + // The following list of selectors is by increasing specificity: + extern const unsigned long long Specificity_Universal; + extern const unsigned long long Specificity_Type; + extern const unsigned long long Specificity_Class; + extern const unsigned long long Specificity_Attr; + extern const unsigned long long Specificity_Pseudo; + extern const unsigned long long Specificity_ID; + extern const unsigned long long Specificity_Inline; // sass keywords extern const char at_root_kwd[]; diff --git a/debugger.hpp b/debugger.hpp index 75dfdfbf6b..5dc486e2ce 100644 --- a/debugger.hpp +++ b/debugger.hpp @@ -2,6 +2,7 @@ #define SASS_DEBUGGER_H #include +#include #include "ast_fwd_decl.hpp" using namespace std; @@ -25,9 +26,15 @@ inline string prettyprint(const string& str) { return clean; } +inline string longToHex(long long t) { + std::stringstream is; + is << std::hex << t; + return is.str(); +} + inline void debug_ast(AST_Node* node, string ind = "", Env* env = 0) { - + if (node == 0) return; if (ind == "") cerr << "####################################################################\n"; if (dynamic_cast(node)) { Bubble* bubble = dynamic_cast(node); @@ -64,6 +71,7 @@ inline void debug_ast(AST_Node* node, string ind = "", Env* env = 0) Complex_Selector* selector = dynamic_cast(node); cerr << ind << "Complex_Selector " << selector << " [block:" << selector->last_block() << "]" + << " [weight:" << longToHex(selector->specificity()) << "]" << (selector->last_block() && selector->last_block()->is_root() ? " [root]" : "") << " [@media:" << selector->media_block() << "]" << (selector->is_optional() ? " [is_optional]": " -") @@ -80,14 +88,15 @@ inline void debug_ast(AST_Node* node, string ind = "", Env* env = 0) debug_ast(selector->tail(), ind + "-", env); } else if (dynamic_cast(node)) { Compound_Selector* selector = dynamic_cast(node); - cerr << ind << "Compound_Selector " << selector - << " [block:" << selector->last_block() << "]" - << (selector->last_block() && selector->last_block()->is_root() ? " [root]" : "") - << " [@media:" << selector->media_block() << "]" - << (selector->is_optional() ? " [is_optional]": " -") - << (selector->has_line_break() ? " [line-break]": " -") - << (selector->has_line_feed() ? " [line-feed]": " -") << - " <" << prettyprint(selector->pstate().token.ws_before()) << ">" << endl; + cerr << ind << "Compound_Selector " << selector; + cerr << " [block:" << selector->last_block() << "]"; + cerr << " [weight:" << longToHex(selector->specificity()) << "]"; + // cerr << (selector->last_block() && selector->last_block()->is_root() ? " [root]" : ""); + cerr << " [@media:" << selector->media_block() << "]"; + cerr << (selector->is_optional() ? " [is_optional]": " -"); + cerr << (selector->has_line_break() ? " [line-break]": " -"); + cerr << (selector->has_line_feed() ? " [line-feed]": " -"); + cerr << " <" << prettyprint(selector->pstate().token.ws_before()) << ">" << endl; for(auto i : selector->elements()) { debug_ast(i, ind + " ", env); } } else if (dynamic_cast(node)) { Propset* selector = dynamic_cast(node); diff --git a/extend.cpp b/extend.cpp index 7b93aa5ad2..3e28ae85e0 100644 --- a/extend.cpp +++ b/extend.cpp @@ -517,14 +517,14 @@ namespace Sass { // had an extra source that the ruby version did not have. Without a failing test case, this is going to be extra hard to find. My // best guess at this point is that we're cloning an object somewhere and maintaining the sources when we shouldn't be. This is purely // a guess though. - int maxSpecificity = 0; + unsigned long long maxSpecificity = 0; SourcesSet sources = pSeq1->sources(); DEBUG_PRINTLN(TRIM, "TRIMASDF SEQ1: " << seq1) DEBUG_EXEC(TRIM, printSourcesSet(sources, ctx, "TRIMASDF SOURCES: ")) for (SourcesSet::iterator sourcesSetIterator = sources.begin(), sourcesSetIteratorEnd = sources.end(); sourcesSetIterator != sourcesSetIteratorEnd; ++sourcesSetIterator) { - const Complex_Selector* const pCurrentSelector = *sourcesSetIterator; + const Complex_Selector* const pCurrentSelector = *sourcesSetIterator; maxSpecificity = max(maxSpecificity, pCurrentSelector->specificity()); }