Skip to content

Commit

Permalink
Merge pull request #3100 from mgreter/bugfix/revert-compound-re-order…
Browse files Browse the repository at this point in the history
…-non-extended

Revert compound re-ordering for non extended selectors
  • Loading branch information
mgreter committed May 21, 2020
2 parents c3c1c0e + e90a9d2 commit f4cd410
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 19 deletions.
33 changes: 29 additions & 4 deletions src/ast_selectors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,16 @@ namespace Sass {
return false;
}

bool ComplexSelector::isInvalidCss() const
{
for (size_t i = 0; i < length(); i += 1) {
if (CompoundSelectorObj compound = get(i)->getCompound()) {
if (compound->isInvalidCss()) return true;
}
}
return false;
}

SelectorListObj ComplexSelector::wrapInList()
{
SelectorListObj selector =
Expand Down Expand Up @@ -525,15 +535,13 @@ namespace Sass {
CompoundSelector::CompoundSelector(SourceSpan pstate, bool postLineBreak)
: SelectorComponent(pstate, postLineBreak),
Vectorized<SimpleSelectorObj>(),
hasRealParent_(false),
extended_(false)
hasRealParent_(false)
{
}
CompoundSelector::CompoundSelector(const CompoundSelector* ptr)
: SelectorComponent(ptr),
Vectorized<SimpleSelectorObj>(*ptr),
hasRealParent_(ptr->hasRealParent()),
extended_(ptr->extended())
hasRealParent_(ptr->hasRealParent())
{ }

size_t CompoundSelector::hash() const
Expand Down Expand Up @@ -950,6 +958,23 @@ namespace Sass {
std::sort(begin(), end(), cmpSimpleSelectors);
}

bool CompoundSelector::isInvalidCss() const
{
size_t current = 0, next = 0;
for (const SimpleSelector* sel : elements()) {
next = sel->getSortOrder();
// Must only have one type selector
if (current == 1 && next == 1) {
return true;
}
if (next < current) {
return true;
}
current = next;
}
return false;
}

/* better return sass::vector? only - is empty container anyway? */
SelectorList* ComplexSelector::resolve_parent_refs(SelectorStack pstack, Backtraces& traces, bool implicit_parent)
{
Expand Down
17 changes: 9 additions & 8 deletions src/ast_selectors.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ namespace Sass {
class ClassSelector final : public SimpleSelector {
public:
ClassSelector(SourceSpan pstate, sass::string n);
int getSortOrder() const override final { return 3; }
int getSortOrder() const override final { return 2; }
virtual unsigned long specificity() const override;
bool operator==(const SimpleSelector& rhs) const final override;
ATTACH_CMP_OPERATIONS(ClassSelector)
Expand Down Expand Up @@ -216,7 +216,7 @@ namespace Sass {
ADD_PROPERTY(char, modifier);
public:
AttributeSelector(SourceSpan pstate, sass::string n, sass::string m, String_Obj v, char o = 0);
int getSortOrder() const override final { return 4; }
int getSortOrder() const override final { return 2; }
size_t hash() const override;
virtual unsigned long specificity() const override;
bool operator==(const SimpleSelector& rhs) const final override;
Expand All @@ -237,7 +237,7 @@ namespace Sass {
ADD_PROPERTY(bool, isClass)
public:
PseudoSelector(SourceSpan pstate, sass::string n, bool element = false);
int getSortOrder() const override final { return 5; }
int getSortOrder() const override final { return 3; }
virtual bool is_pseudo_element() const override;
size_t hash() const override;

Expand Down Expand Up @@ -273,16 +273,17 @@ namespace Sass {
// Between each item there is an implicit ancestor of combinator
////////////////////////////////////////////////////////////////////////////
class ComplexSelector final : public Selector, public Vectorized<SelectorComponentObj> {
ADD_PROPERTY(bool, chroots)
ADD_PROPERTY(bool, chroots);
// line break before list separator
ADD_PROPERTY(bool, hasPreLineFeed)
ADD_PROPERTY(bool, hasPreLineFeed);
public:
ComplexSelector(SourceSpan pstate);

// Returns true if the first components
// is a compound selector and fullfills
// is a compound selector and fulfills
// a few other criteria.
bool isInvisible() const;
bool isInvalidCss() const;

size_t hash() const override;
void cloneChildren() override;
Expand Down Expand Up @@ -413,12 +414,11 @@ namespace Sass {
////////////////////////////////////////////////////////////////////////////
class CompoundSelector final : public SelectorComponent, public Vectorized<SimpleSelectorObj> {
ADD_PROPERTY(bool, hasRealParent)
ADD_PROPERTY(bool, extended)
public:
CompoundSelector(SourceSpan pstate, bool postLineBreak = false);

// Returns true if this compound selector
// fullfills various criteria.
// fulfills various criteria.
bool isInvisible() const;

bool empty() const override {
Expand Down Expand Up @@ -454,6 +454,7 @@ namespace Sass {
bool operator==(const SimpleSelector& rhs) const;

void sortChildren();
bool isInvalidCss() const;

ATTACH_CMP_OPERATIONS(CompoundSelector)
ATTACH_AST_OPERATIONS(CompoundSelector)
Expand Down
29 changes: 28 additions & 1 deletion src/extender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,20 @@ namespace Sass {
sass::vector<Extension> exts = options[0];
for (size_t n = 0; n < exts.size(); n += 1) {
exts[n].assertCompatibleMediaContext(mediaQueryContext, traces);
// To fix invalid css we need to re-order some
// Therefore we need to make copies for them
if (exts[n].extender->isInvalidCss()) {
exts[n].extender = SASS_MEMORY_COPY(exts[n].extender);
for (SelectorComponentObj& component : exts[n].extender->elements()) {
if (CompoundSelector* compound = component->getCompound()) {
if (compound->isInvalidCss()) {
CompoundSelector* copy = SASS_MEMORY_COPY(compound);
copy->sortChildren();
component = copy;
}
}
}
}
result.push_back(exts[n].extender);
}
return result;
Expand Down Expand Up @@ -844,10 +858,23 @@ namespace Sass {
}

for (sass::vector<SelectorComponentObj>& components : complexes) {
auto sel = SASS_MEMORY_NEW(ComplexSelector, "[ext]");
auto sel = SASS_MEMORY_NEW(ComplexSelector, "[unified]");
sel->hasPreLineFeed(lineBreak);
sel->elements(components);

/* This seems to do too much in regard of previous behavior
for (SelectorComponentObj& component : sel->elements()) {
if (CompoundSelector* compound = component->getCompound()) {
if (compound->isInvalidCss()) {
CompoundSelector* copy = SASS_MEMORY_COPY(compound);
copy->sortChildren();
component = copy;
}
}
}*/

unifiedPaths.push_back(sel);

}

}
Expand Down
11 changes: 5 additions & 6 deletions src/inspect.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ namespace Sass {
if (list->is_bracketed()) {
append_string(lbracket(list));
}
// probably ruby sass eqivalent of element_needs_parens
// probably ruby sass equivalent of element_needs_parens
else if (output_style() == TO_SASS &&
list->length() == 1 &&
!list->from_selector() &&
Expand Down Expand Up @@ -489,7 +489,7 @@ namespace Sass {
}
append_string(rbracket(list));
}
// probably ruby sass eqivalent of element_needs_parens
// probably ruby sass equivalent of element_needs_parens
else if (output_style() == TO_SASS &&
list->length() == 1 &&
!list->from_selector() &&
Expand Down Expand Up @@ -1017,7 +1017,7 @@ namespace Sass {


bool was_comma_array = in_comma_array;
// probably ruby sass eqivalent of element_needs_parens
// probably ruby sass equivalent of element_needs_parens
if (output_style() == TO_SASS && g->length() == 1 &&
(!Cast<List>((*g)[0]) &&
!Cast<SelectorList>((*g)[0]))) {
Expand Down Expand Up @@ -1045,7 +1045,7 @@ namespace Sass {
}

in_comma_array = was_comma_array;
// probably ruby sass eqivalent of element_needs_parens
// probably ruby sass equivalent of element_needs_parens
if (output_style() == TO_SASS && g->length() == 1 &&
(!Cast<List>((*g)[0]) &&
!Cast<SelectorList>((*g)[0]))) {
Expand Down Expand Up @@ -1081,7 +1081,7 @@ namespace Sass {
void Inspect::operator()(SelectorComponent* sel)
{
// You should probably never call this method directly
// But in case anyone does, we will do the upcasting
// But in case anyone does, we will do the up-casting
if (auto comp = Cast<CompoundSelector>(sel)) operator()(comp);
if (auto comb = Cast<SelectorCombinator>(sel)) operator()(comb);
}
Expand All @@ -1091,7 +1091,6 @@ namespace Sass {
if (sel->hasRealParent()) {
append_string("&");
}
sel->sortChildren();
for (auto& item : sel->elements()) {
item->perform(this);
}
Expand Down

0 comments on commit f4cd410

Please sign in to comment.