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

Already on GitHub? Sign in to your account

Firefox Ignores font-size in font_reset mixin. #852

Closed
farmdawgnation opened this Issue Apr 19, 2012 · 10 comments

Comments

Projects
None yet
2 participants
Contributor

farmdawgnation commented Apr 19, 2012

I'm talking about the mixin located here:
https://github.com/chriseppstein/compass/blob/stable/frameworks/compass/stylesheets/compass/reset/_utilities.scss#L69

It seems that when you specify font-size followed by font, Firefox ignores the shorthand property overrides the font-size property, causing it to be ignored, as shown by this Firebug screenshot:

Example of Issue

This was tested against Firefox 11, Windows and Mac versions. Let me know if I can provide you guys with any more details.

Owner

mirisuzanne commented Apr 19, 2012

Thanks! Are you willing to do some tests and submit a patch?

I'd expect that to work if you switched the order of the two properties.

Contributor

farmdawgnation commented Apr 19, 2012

I am more than happy to do that!

Owner

mirisuzanne commented Apr 19, 2012

Thanks again!

Contributor

farmdawgnation commented Apr 19, 2012

So, for some reason reversing these causes the assert_renders_correctly test to fail for blueprint. Any thoughts on that? Heh.

Owner

mirisuzanne commented Apr 19, 2012

you're changing the output of a common mixin, so the change may show up in several tests. Check the css output files in the blueprint tests, and change the order of the expected output. Does that make sense?

Contributor

farmdawgnation commented Apr 19, 2012

Aha. I was wondering how you guys would check for correct rendering. Having a cached copy makes sense. Found it and changed and now that test is passing. I'll proceed to make sure the fix actually works in Firefox.

As a side point to this, I don't know if you're aware of this or not but the stable branch tests are currently throwing a stack size error as well. It was happening before my change so it wasn't introduced by me, but just thought you should know if you didn't already.

Owner

mirisuzanne commented Apr 19, 2012

Care to file a bug about the failing test while you're at it? Thanks! :)

Contributor

farmdawgnation commented Apr 19, 2012

Your fix worked as expected. Pull request opened. :)

Contributor

farmdawgnation commented Apr 19, 2012

Also, yes, I can file a bug about the stack error.

mirisuzanne added a commit that referenced this issue Apr 19, 2012

Merge pull request #853 from farmdawgnation/stable
[#852] Fix ordering of font properties in reset-font for FF's benefit.
Owner

mirisuzanne commented Apr 19, 2012

thanks, and thanks. great work.

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