Anyone got a magnifying glass?
-
Better, but still doesn't actually address the root cause...
Exactly. The root cause looks to me to be their SCSS parser doing math on what should just be written out (because they didn't escape correctly). Obvious, because now it's put in the CSS:
html { font-size: 0.73684; }
Which is what 14 / 19 comes out to be.
Grabbed this screenshot *after* the most recent update.EDIT: Changed my pinpointing above to what I actually meant.
-
Exactly. The root cause looks to me to be their minifier deciding to do math on what should just be written out. Obvious, because now it's put in the CSS:
So when's the front-page article on this?
-
The root cause looks to me to be their minifier deciding to do math on what should just be written out.
The real root cause looks like the line from that Github snippet above:
html {font-size: $base-font-size/$base-line-height;}
Maybe I'm not Modern enough for webdev, but I think this is insane.
-
Looks like someone confused
font
withfont-size
?html {font: 16px/24px serif;}
is valid CSS.
html {font-size: 16px/24px;}
isn't.
-
The snippet is from a .scss file so I assume it's at least correct for that. However in terms of making actual sense, I would say that's not a good way to define a font size.
-
The real root cause looks like the line from that Github snippet above
That, actually, is closer to what I meant (what I actually meant was what is merging the SCSS files to CSS files, since it looks like it would try to write strings in those fields, unless I'm grossly misreading it), but I'm being stretched thin today so I'm not all here and trying to put up stuff fast.
Looks like someone confused font with font-size?
If it actually output the latter, I think we wouldn't be having the issue, as I'm pretty sure every browser would just ignore it as being invalid.
-
Exactly. The root cause looks to me to be their minifier deciding to do math on what should just be written out. Obvious, because now it's put in the CSS:
The real root cause looks like the line from that Github snippet above:
html {font-size: $base-font-size/$base-line-height;}
Maybe I'm not Modern enough for webdev, but I think this is insane.
Actually, the root cause is bad CSS. They're trying to take something that is valid in
font:
and use it infont-size:
.In the
font:
property, the {unit}/{unit} construct is used to specify font size and line height. It appears that they are trying to apply this same construct tofont-size:
. This is not a valid construct. As a result, the values are apparently getting divided (sometimes), which is what leads to this issue.cc: @sam, @codinghorror
Edit: Replaced an errant period with a comma.
-
If it actually output the latter, I think we wouldn't be having the issue, as I'm pretty sure every browser would just ignore it as being invalid.
In any case, some moron tried to specify both
font-size
andline-height
on one line using a shorthand syntax that both doesn't work and confuses the SCSS parser.Edit: Damn, @abarker was faster.
-
-
Actually, the root cause is bad CSS.
I think it's a mix of the two, going from information gleaned:
In the SCSS:
html {font-size: $base-font-size/$base-line-height;}
In the diff that @PJH posted:
+$base-font-size: 14px !default; +$base-line-height: 19px !default;
So, what it should be trying to put is (note, I lack knowledge for how SCSS works specifically, so this is what I'm guessing it should put out):
html {font-size: 14px !default/19px !default;}
But, for some reason, decides to convert them to numbers only, then parse as a mathematical conversion, and gives us:
html {font-size: 0.73684;}
Which boils down to a bug in the SCSS parser and trying to do invalid CSS that combine into a
-
I think it's a mix of the two, going from information gleaned:
*cough*
As a result, the values are apparently getting divided (sometimes), which is what leads to this issue.
-
Which boils down to a bug in the SCSS parser and trying to do invalid CSS that combine into a
Are you sure that's a bug? Because according to http://sass-lang.com/guide, you're supposed to be able to write crazy stuff like
width: 300px / 960px * 100%;
in SCSS.
BTW: Discourse removes trailing Emoji from quotes. Paging @discoursebot.
-
*cough*
Reading is a barrier to posting conjecture and pointing out flaws in the Discourse software.
Also, I'm pretty sure it's not "sometimes" but "all the time" that it's dividing the number in the CSS. It's just Firefox that sometimes applies the bad CSS:
-
@asdf - Days Since Last Discourse Bug: -1
-
Are you sure that's a bug?
note, I lack knowledge for how SCSS works specifically
you're supposed to be able to write crazy stuff like
Then I guess it's a case of developers (the Discourse ones) not having a clue how their tools work, compounded by not testing appropriately. But then, if they wanted to do the right shorthand method, how would they write it and not have the SCSS parser help them by converting it?
BTW: Discourse removes trailing Emoji from quotes
It's by design that quotes strip all formatting, including images and emoji.
-
It's by design that quotes strip all formatting, including images and emoji.
WTF? Are you trying to tell me that's not a bug? The sentence I quoted above would have made absolutely no sense withouth the :heavy_exclamation_mark:
-
how would they write it and not have the SCSS parser help them by converting it?
Ah, looked into the documentation and found my answer:
If you want to use variables along with a plain CSS /, **you can use #{} to insert them.**
Emphasis mine.
-
Are you trying to tell me that's not a bug?
According to them. We reported it as such early on and it was dismissed, even making the argument that stripping formatting also can strip context.
-
If you want to use variables along with a plain CSS /, you can use #{} to insert them. For example:
Discourse'd! Looks like it removed more than just some emoji from your post. :D
-
It's by design that quotes strip all formatting, including images and emoji.
Quoting is Discoursistent in that highlight-quoting strips out all that stuff, but full-quoting from the reply pane retains it.
-
Looks like it removed more than just some emoji from your post.
It looks like it wasn't there to start with. For future reference, the context:
If you want to use variables along with a plain CSS /, you can use #{} to insert them. For example:
p { $font-size: 12px; $line-height: 30px; font: #{$font-size}/#{$line-height}; } ```</blockquote>
-
full-quoting from the reply pane retains it.
But escapes any HTML entities, so images aren't actually quoted unless the HTML is manually fixed.
-
Discoursistency!
-
Also, I'm pretty sure it's not "sometimes" but "all the time" that it's dividing the number in the CSS.
I'll give you that one.
-
Exactly. The root cause looks to me to be their SCSS parser deciding to do math on what should just be written out. Obvious, because now it's put in the CSS:
html { font-size: 0.73684; }
Which is what 14 / 19 comes out to be.
Exactly.@codinghorror
Stop.
Fucking.
Hacking.
Blindly.
Fix.
The.
Fucking.
Bug.
-
Exactly.
@codinghorror
Stop.
Fucking.
Hacking.
Blindly.
Fix.
The.
Fucking.
Bug.Keep reading! You're almost to the real, real root cause!
-
Wait, hacking blindly isn't the real, real root cause?
[size=9]Also, I edited that post again to be more clear about the root cause.[/size]
-
But it's real real REALLY real root causes all the way down! Ayieeee!
-
But it's real real REALLY real root causes all the way down! Ayieeee!
show me the Nth root cause and i shall tell you to find the (N+1)th root cause.
-
Shouldn't that be, "And finding the Nth + 1 root cause is left as an exercise to the
readerusers."
-
i am in the presence of a master!
-
:bow:
- Looks more like a guy peacocking to me. Either that, or he's wearing a tiara.
-
This bug makes me want to go rage-develop a patch. Did someone slip a PRNG into the CSS generator?
WTFH!
-
Seems the newest upgrade fixed it?
http://what.thedailywtf.com/t/docker-upgrades/1929/117?u=jbert
-
Shouldn't that be, "And finding the Nth + 1 root cause is left as an exercise to the <strike>reader</strike> users."
It'd be easier if we could just find loge(cause) and work out the rest from that.
-
Seems the newest upgrade fixed it?
Aaaand of course, the commit log blames Firefox instead of incorrect CSS
-
Aaaand of course, the commit log blames Firefox instead of incorrect CSS
I thought it was more something between supposedly syntactically correct SCSS and whatever parser they're using to create the CSS...
-
I thought it was more something between supposedly syntactically correct SCSS and whatever parser they're using to create the CSS...
Either way, it's typical of the attitude Atwood shows to blame Firefox for obeying a fucked up CSS rule instead of blaming the fucked up CSS rule itself.At least this time he's actually fixed the damn bug. Only took, what, three, four attempts?
-
Is this all why Chrome is cramping all the lines of messages together now?
-
Is this all why Chrome is cramping all the lines of messages together now?
Now that they managed to get syntactically correct CSS, they can start debugging the actual consequences of the line-height changes...
-
Would you check to make sure it's nothing I've done on the site-css?
Add
?preview-style=5ac5a5e9-c6ed-4197-9ec3-40811fd6ee5d
to the URL - if you have a blue background, then that is the only non DC CSS being applied (just so you can tell it is actually being applied.)The preview-style lasts, from memory, in the tab you've applied it to until you press back or manually change the URL again. Clicking links should retain it.
-
Example:
-
Is this all why Chrome is cramping all the lines of messages together now?
Well, the changeset fixed thefont-size
bug, but it got rid of theline-height
info...Discourse: We fix bugs with bugs
-
Is there a reason why they didn't just specify
line-height
separately?
-
Trying to be clever and squeeze everything into the shorthand version of a single property?
-
Is there a reason why they didn't just specify
line-height
separately?
Because that would be the correct thing to do.
Actually, a better reason would be because they don't fucking test in one of the top three browsers by market share.
-
Did someone slip a PRNG into the CSS generator?
No. Read the the posts up-thread to determine what is happening.
-
Is there a reason why they didn't just specify line-height separately?
Because they didn't read the CSS spec correctly and tried to use a shorthand incorrectly.
-
Because they didn't read the CSS spec correctly and tried to use a shorthand incorrectly.
I think it's their preprocessor (or whatever SASS is):
I guess it's only fair, since discourse posts have problems escaping stuff correctly that the discourse source code have similar problems.
-
I think it's their preprocessor
Which, as we discovered, was working as intended and designed. The variables weren't being escaped as the documentation states they should be, so the SASS processor worked as specified. And that link shows that they don't know how their tools even work, especially the caveat with using CSS shorthand and variables.
The long and short of the bug is:
- The Discourse devs tried using a CSS shorthand for the wrong element (
font
supports a shorthand version,font-size
does not). - The Discourse devs didn't escape the variables per the documentation in the SCSS files, so the SASS processor worked as designed.
- The combination revealed a bug in Firefox where it sometimes tries to help out and made the font-size that was missing a type specifier in pixels.
Also, great, so I read the topic and see Atwood chime in without actually doing any debugging, so they changed something that was working fine when they also removed what wasn't (as the bad
font-size
was added in a separate section of the CSS and not that main one).That's just
- The Discourse devs tried using a CSS shorthand for the wrong element (