Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deep @extend specificity problems - continuation of the continuation #1248

Closed
mgol opened this issue May 28, 2015 · 5 comments · Fixed by #1501
Closed

Deep @extend specificity problems - continuation of the continuation #1248

mgol opened this issue May 28, 2015 · 5 comments · Fixed by #1501

Comments

@mgol
Copy link

mgol commented May 28, 2015

Refs #1091 #592

The following SCSS:

.a.b .c {
  top: 0;
}
.a {
  @extend .b;
}
.a .d {
  @extend .c;
}

is converted by Ruby Sass 3.4.14 to:

.a.b .c, .a .c, .a.b .d, .a .d {
  top: 0; }

libsass 3.2.4 produces:

.a.b .c, .a .c, .a .d {
  top: 0; }

The .a.b .d selector is missing, it's important due to CSS specificity.

EDIT: test case simplified.

@mgol
Copy link
Author

mgol commented May 28, 2015

Spec PR: sass/sass-spec#399

@xzyfer
Copy link
Contributor

xzyfer commented May 31, 2015

Thanks @mzgol we're looking into it.

mgol added a commit to mgol/sass-spec that referenced this issue Jun 1, 2015
@mgreter mgreter modified the milestones: 3.3.1, 3.3, 3.4 Jun 13, 2015
@mgreter
Copy link
Contributor

mgreter commented Jun 16, 2015

Hey @mzgol first of all thanks for the test case! I tried to dig into the problem, as I thought with my latest refactor I had enough knowledge of most code to understand it. With the help of this test case and by comparing the output of ruby sass, I was able to narrow down the problem, but unfortunately my knowledge is not enough to have it fixed (@xyfer maybe you know more about the internals of extend, specially in regards to sources attached to compound selector, haven't really seen that one before?).

So I thought I document my findings here. First the code files involved as reference:

  • extend.cpp (main extend code)
  • node.cpp (crutch for extend?)
  • sass_util.cpp (flatten and paths)
  • ast.cpp (ast node related stuff)

There are a few todo notes in extend and a few indicate in the right direction IMHO. For me it looks like a compound selector in the second extend has incorrect sources. At least that's how far I got by comparing ruby sass to libsass. I attached two raw debug outputs of the test case, which does show a few differences (sorry, don't know any free and good compare view hoster and was too lazy to create something on github for this, just copy pase into your favourite compare tool 😉 ).

Ruby:

+++++++++++++++++++++++++ .a
extended_not_expanded: [[[.a]]]
paths: [[[.a]]]
++TRIM: [[[.a]]]
SEQS1: [[.a]]
SRCS: #<Set: {.a}>
max spec 1000
SEQS2: [[.a]]
--TRIMMED: [[[.a]]]
[.a]
++TRIM: [[[.a]]]
SEQS1: [[.a]]
SRCS: #<Set: {.a}>
max spec 1000
SEQS2: [[.a]]
--TRIMMED: [[[.a]]]
+++++++++++++++++++++++++ .a.b
DO EXTEND
DO EXTEND
+++++++++++++++++++++++++ .a
DO EXTEND
+++++++++++++++++++++++++ .d
extended_not_expanded: [[[.a]], [[.d]]]
paths: [[[.a], [.d]]]
++TRIM: [[[.a, .d]]]
SEQS1: [[.a, .d]]
SRCS: #<Set: {.a .d}>
max spec 2000
SEQS2: [[.a, .d]]
--TRIMMED: [[[.a, .d]]]
[.a .d]
++TRIM: [[[.a, .d]]]
SEQS1: [[.a, .d]]
SRCS: #<Set: {.a .d}>
max spec 2000
SEQS2: [[.a, .d]]
--TRIMMED: [[[.a, .d]]]
+++++++++++++++++++++++++ .c
extended_not_expanded: [[[.a.b], [.a]], [[.c], [.a, .d]]]
paths: [[[.a.b], [.c]], [[.a], [.c]], [[.a.b], [.a, .d]], [[.a], [.a, .d]]]
SUBW SEQ1: [.a.b]
SUBW SEQ2: [.a]
INNER OPS1: []
INNER OPS2: []
table: [[0]]
x: [nil]
y: [nil]
lcs: []
MERGED: []
SUBW INIT: []
SUBW FIN: []
SUBW TWO SEQ1: [[.a.b]]
SUBW TWO SEQ2: [[.a]]
table: [[0, 0], [0, 1]]
x: [nil, [.a]]
y: [nil, [.a.b]]
lcs: [[.a.b]]
RESULT: [[.a.b]]
SUBW SEQ1: [.a]
SUBW SEQ2: [.a]
INNER OPS1: []
INNER OPS2: []
table: [[0]]
x: [nil]
y: [nil]
lcs: []
MERGED: []
SUBW INIT: []
SUBW FIN: []
SUBW TWO SEQ1: [[.a]]
SUBW TWO SEQ1: [[.a]]
table: [[0, 0], [0, 1]]
x: [nil, [.a]]
y: [nil, [.a]]
lcs: [[.a]]
RESULT: [[.a]]
++TRIM: [[[.a.b, .c]], [[.a, .c]], [[.a.b, .d]], [[.a, .d]]]
SEQS1: [[.a.b, .c]]
SRCS: #<Set: {.a.b .c}>
max spec 3000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
SEQS1: [[.a, .c]]
SRCS: #<Set: {.a, .a.b .c}>
max spec 3000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
SEQS1: [[.a.b, .d]]
SRCS: #<Set: {.a.b .c, .a .d}>
max spec 3000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
SEQS1: [[.a, .d]]
SRCS: #<Set: {.a .d}>
max spec 2000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
--TRIMMED: [[[.a.b, .c]], [[.a, .c]], [[.a.b, .d]], [[.a, .d]]]
[.a.b .c, .a .c, .a.b .d, .a .d]
++TRIM: [[[.a.b, .c]], [[.a, .c]], [[.a.b, .d]], [[.a, .d]]]
SEQS1: [[.a.b, .c]]
SRCS: #<Set: {.a.b .c}>
max spec 3000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
SEQS1: [[.a, .c]]
SRCS: #<Set: {.a, .a.b .c}>
max spec 3000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
SEQS1: [[.a.b, .d]]
SRCS: #<Set: {.a.b .c, .a .d}>
max spec 3000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
SEQS1: [[.a, .d]]
SRCS: #<Set: {.a .d}>
max spec 2000
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
--TRIMMED: [[[.a.b, .c]], [[.a, .c]], [[.a.b, .d]], [[.a, .d]]]
.a.b .c, .a .c, .a.b .d, .a .d {
  top: 0; }

Libsass

+++++++++++++++++++++++++ SOURCES WITH NEW SOURCE: SourcesSet[[.a]]
EXTENDED NOT EXPANDED: [[[.a]]]
PATHS: [[[.a]]]
SUB: [[]]
WEAVED: [[[.a]]]
++TRIM: [[[.a]]]
SEQS1: [.a]
adding from head
SEQ1 SOURCES: SourcesSet[[.a]]
MAX SPECIFICITY: 65536
SEQS2: [[.a]]
PUSHING: [.a]
--TRIMMED: [[[.a]]]
+++++++++++++++++++++++++ SOURCES THIS EXTEND: SourcesSet[]
+++++++++++++++++++++++++ SOURCES WITH NEW SOURCE: SourcesSet[[.a, .d]]
EXTENDED NOT EXPANDED: [[[.a]], [[.d]]]
PATHS: [[[.a], [.d]]]
SUB: [[]]
SUB: [[.a]]
WEAVED: [[[.a, .d]]]
++TRIM: [[[.a, .d]]]
SEQS1: [.a, .d]
SEQ1 SOURCES: SourcesSet[[.a, .d]]
MAX SPECIFICITY: 131072
SEQS2: [[.a, .d]]
PUSHING: [.a, .d]
EXTENDED NOT EXPANDED: [[[.a.b], [.a]], [[.c], [.a, .d]]]
PATHS: [[[.a.b], [.c]], [[.a], [.c]], [[.a.b], [.a, .d]], [[.a], [.a, .d]]]
SUB: [[]]
SUB: [[.a.b]]
SUB: [[]]
SUB: [[.a]]
SUB: [[]]
SUBW seq1: [.a.b]
SUBW seq2: [.a]
INNER OPS1: []
INNER OPS2: []
VERY VERY VERY VERY SMELLY
LCS CALLED WITH NOW: X=[nil] Y=[nil]
LCSTABLE: X=[nil] Y=[nil]
LCSBACK_IN: X=[nil] Y=[nil] I=0 J=0
RETURNING EMPTY
INIT: []
FIN: []
SUBW TWO SEQ1: [[.a.b]]
SUBW TWO SEQ2: [[.a]]
got from her
LCSSS: X=0x2e0db80,  Y=0x2e0da40,
RETURNING AFTER ELEM COMPARE
RETURNING EMPTY
LCS: table=:0:0,:0:1,
SEQLCS: [[.a.b]]
FLATTENED: [[.a.b]]
SUB: [[.a.b]]
SUB: [[]]
SUBW seq1: [.a]
SUBW seq2: [.a]
INNER OPS1: []
INNER OPS2: []
VERY VERY VERY VERY SMELLY
LCS CALLED WITH NOW: X=[nil] Y=[nil]
LCSTABLE: X=[nil] Y=[nil]
LCSBACK_IN: X=[nil] Y=[nil] I=0 J=0
RETURNING EMPTY
INIT: []
FIN: []
SUBW TWO SEQ1: [[.a]]
SUBW TWO SEQ2: [[.a]]
got from her
LCSSS: X=0x2e0e580,  Y=0x2e0e440,
RETURNING AFTER ELEM COMPARE
RETURNING EMPTY
LCS: table=:0:0,:0:1,
SEQLCS: [[.a]]
FLATTENED: [[.a]]
SUB: [[.a]]
WEAVED: [[[.a.b, .c]], [[.a, .c]], [[.a.b, .d]], [[.a, .d]]]
++TRIM: [[[.a.b, .c]], [[.a, .c]], [[.a.b, .d]], [[.a, .d]]]
SEQS1: [.a.b, .c]
####################################################################
Complex_Selector 0x2e0ec60 (18446744073709551615@[0:0]-[0:0]) [weight:30000] [@media:0] - - - --  <>
 Compound_Selector 0x2e121c0 (18446744073709551615@[0:0]-[0:0]) [weight:0] [@media:0] - - - <>
  Parent_Selector 0x2e19460 (18446744073709551615@[0:0]-[0:0]) <>
{ }Complex_Selector 0x2e0ed00 (0@[0:2]-[0:4]) [weight:30000] [@media:0] - - - --  <>
{ } Compound_Selector 0x2c2e760 (0@[0:0]-[0:0]) [weight:20000] [@media:0] - - - <>
{ }  Selector_Qualifier 0x29b46a0 (0@[0:0]-[0:2]) <<.a>> - -
{ }  Selector_Qualifier 0x29b4730 (0@[0:2]-[0:4]) <<.b>> - -
{ }{ }Complex_Selector 0x2e0eda0 (0@[0:5]-[0:7]) [weight:10000] [@media:0] - - - --  <>
{ }{ } Compound_Selector 0x2c2e840 (0@[0:2]-[0:4]) [weight:10000] [@media:0] - - - <>
{ }{ }  Selector_Qualifier 0x29b47c0 (0@[0:5]-[0:7]) <<.c>> - -
####################################################################
Abort - something smells fishy - seqs1 has no sources
SEQ1 SOURCES: SourcesSet[]
MAX SPECIFICITY: 0
SEQS2: [[.a.b, .c]]
SEQS2: [[.a, .c]]
FOUND MORE SPECIFIC
YEPP MORE SPECIFIC
SEQS1: [.a, .c]
adding from head
adding from head
SEQ1 SOURCES: SourcesSet[[.a]]
MAX SPECIFICITY: 65536
SEQS2: []
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
PUSHING: [.a, .c]
SEQS1: [.a.b, .d]
adding from head
adding from head
SEQ1 SOURCES: SourcesSet[[.a, .d]]
MAX SPECIFICITY: 131072
SEQS2: []
SEQS2: [[.a, .c]]
SEQS2: [[.a.b, .d]]
SEQS2: [[.a, .d]]
FOUND MORE SPECIFIC
YEPP MORE SPECIFIC
SEQS1: [.a, .d]
adding from head
adding from head
SEQ1 SOURCES: SourcesSet[[.a, .d]]
MAX SPECIFICITY: 131072
SEQS2: []
SEQS2: [[.a, .c]]
SEQS2: []
SEQS2: [[.a, .d]]
PUSHING: [.a, .d]
--TRIMED: [[], [[.a, .c]], [], [[.a, .d]]]
.a.b .c, .a .c, .a .d {
  top: 0; }

Following things seem to be wrong:

  • SEQS1: [.a.b, .c] has no sources and therefore a specificity of 0
  • ... which will lead to the removal of said selector (hence this bug)
  • last extend has different SourceSet for every sequence
  • ... libsass: [], [.a], [.a, .d], [.a, .d] vs ruby: {.a.b .c}, {.a, .a.b .c}, {.a.b .c, .a .d}, {.a .d}
  • we miss a ++++ SOURCES WITH NEW SOURCE early in libsass

Maybe @akhleung or @xzyfer have more ideas what could be going wrong!? If it is of any use, I could also provide the modifications for ruby sass to get the debug output as posted above.

mgreter added a commit to mgreter/libsass that referenced this issue Aug 30, 2015
@mgreter mgreter modified the milestones: 3.3, 3.4 Aug 30, 2015
@mgreter mgreter self-assigned this Aug 30, 2015
@mgreter
Copy link
Contributor

mgreter commented Aug 30, 2015

Probably one of the most occult things I have ever debugged. But finally after probably the third different stab at it: "mission impossible accomplished" -> #1501

@mgol
Copy link
Author

mgol commented Aug 31, 2015

@mgreter Thanks! \o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants