Download & Extend
Advanced CSS/JS Aggregation » Issues

CSSTidy - @font-face broken in Firefox 3.6.x. & Opera 11.1x

Posted by melon on April 7, 2011 at 11:41pm
Project:Advanced CSS/JS Aggregation
Version:6.x-1.x-dev
Component:CSS Compression
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (won't fix)

Issue Summary

Hi Mikey,

I found that the csstidy library included in advagg does have the fix for multiple @font-face src and format declarations, but the fix doesn't work in php 5.3.x, because the strips out @font-face src and format strings.

in class.csstidy.inc, line 764 I changed:
foreach($this->sub_value_arr as &$sub_value) {
to:
foreach($this->sub_value_arr as $sub_value) {

And it seems to work for me. I don't know if this breaks anything else. Sorry, no patch this time ;)

Login or register to post comments

Comments

#1

Posted by mikeytown2 on April 8, 2011 at 12:27am
Status:active» fixed

Sent a message to the author https://github.com/oroboto/CSSTidy

This patch has been committed.

AttachmentSize
advagg-csstidy-1120266-1.patch 739 bytes

#2

Posted by melon on April 11, 2011 at 11:36am
Status:fixed» needs work

This still needs work as it works on Chrome and IE but still not working on Firefox/3.6.x.
Will look into it soon.

#3

Posted by mikeytown2 on April 12, 2011 at 7:41pm
Title:CSSTidy breaks @font-face in PHP5.3» CSSTidy - @font-face broken in firefox 3.6.x.

#4

Posted by mori on April 21, 2011 at 5:46pm

subscribe

#5

Posted by obrienmd on May 3, 2011 at 3:42am

Broken for me in Opera latest and Firefox 3.6.12. @font-face fonts do not show up.

#6

Posted by mikeytown2 on May 3, 2011 at 4:05am
Title:CSSTidy - @font-face broken in firefox 3.6.x.» CSSTidy - @font-face broken in Firefox 3.6.x. & Opera 11.1x

#7

Posted by obrienmd on May 3, 2011 at 4:22am

Thanks, mikeytown... too lazy to check my own Opera window, for shame :)

#8

Posted by mikeytown2 on May 9, 2011 at 9:04pm
Status:needs work» active

no code to review so moving this to active.

#9

Posted by obrienmd on May 9, 2011 at 11:01pm

Given the CSSTidy github repo isn't very active, wondering if there's a better utility to be using for advagg?

#10

Posted by mikeytown2 on May 9, 2011 at 11:42pm

http://developer.yahoo.com/yui/compressor/css.html
YUI Compressor, but that requires java to be available on the local server.

https://github.com/yui/yuicompressor/blob/master/ports/js/cssmin.js
is a JavaScript port of yui compressor, so that means I could port it to PHP.

https://github.com/isaacs/cssmin
It's a bunch of regular expressions which is what D6 core already has for the most part; it's the code that yui compressor is based off of.

Best option in my opinion, would be for a Drupal dev to fork CSSTidy and continue the development of it on github.

#11

Posted by webfunkin on May 14, 2011 at 2:13pm

sub

#12

Posted by mikeytown2 on May 19, 2011 at 6:10pm

For those interested, I now include the CSS Compressor as well as CSSTidy in the latest dev. Appears to be about 10x faster and I haven't encountered any CSS errors. Testing of this would be greatly appreciated; if everything come back ok I'll make the new library the default.
#1161870: Include the CSS Compressor

#13

Posted by melon on May 19, 2011 at 9:00pm
Status:active» needs review

Including CSS Compressor was an Excellent idea. I just ran a quick test with Firefox 3.6.17 and Opera 11.11 on Linux (these had a problem for me previously) and IE8 on Windows, and the @font-face problem is gone (at least in my cases)

However I couldn't find any fancy knobs for switching libraries, so I quickly created a patch for that.

AttachmentSize
1120266-css_compression_admin.patch 1.67 KB

#14

Posted by mikeytown2 on May 19, 2011 at 9:28pm
Status:needs review» active

Hmmm thought I included the switching radio logic in that patch; looks like I forgot to. Thanks for the code :) it has been committed, with some added wording; see attached patch.

Moving issue back to active.

AttachmentSize
advagg-1120266-14.patch 1.93 KB

#15

Posted by webfunkin on May 20, 2011 at 5:44am

The @font-face problem still exists for me. I am on Firefox 4.

#16

Posted by melon on May 20, 2011 at 7:19am

I just tested the current -dev with Firefox 4.01 on Windows 7 and it works fine.
I am using the following @font-face syntax in my theme:

@font-face {
  font-family: 'FontName';
  src: url('../fonts/fontfile.eot');
  src: url('../fonts/fontfile.eot?iefix') format('eot'),
    url('../fonts/fontfile.woff') format('woff'),
    url('../fonts/fontfile.ttf') format('truetype'),
    url('../fonts/fontfile.svgz#fontfileX3') format('svg'),
    url('../fonts/fontfile.svg#fontfileX3') format('svg');
  font-weight: normal;
  font-style: normal;
}
.fancy-title {
  font-family: FontName, sans-serif;
}

Can you confirm that you are running your CSS through CSS Compressor? I'd make sure by doing a Master reset after switching compressor engines.

#17

Posted by webfunkin on May 20, 2011 at 9:21am

This is the syntax I am using :

@font-face
{
  font-family:FontName;
  src:local(☺),
url(webfont.woff) format(woff),
url(webfont.ttf) format(truetype),
url(webfont.svg#webfontSuUPmbvY) format(svg);
}

Yes, I did do a master reset after selecting CSS Compressor as the compressing tool.

#18

Posted by philbar on May 20, 2011 at 4:16pm

This is why I did for the CSSTidy project to fix this issue.

http://drupal.org/node/842960

See comment #2, #9, #10

#19

Posted by melon on May 20, 2011 at 6:02pm

@webfunkin: I tried your declarations and found that declaring values without single quotes solves the problem on Firefox 3.6.x (didn't test other browsers)
However I think your syntax isn't optimal. No matter how I variated url and src declarations IE couldn't show the font (even with advagg disabled). I'd consider using a syntax quoted in #16

Edit: _with_, not without.

#20

Posted by webfunkin on May 20, 2011 at 6:28pm

Yes, works like a charm now. Apparently, the syntax I was using is now out-dated. Thanks a lot.

#21

Posted by mikeytown2 on May 20, 2011 at 6:45pm

So this means @font-face works with the CSS Compressor Library correct? If that's the case I'll make it the default then and move this issue to "won't fix".

Sound good?

#22

Posted by mikeytown2 on May 20, 2011 at 7:58pm
Status:active» closed (won't fix)

Marking this issue as "Won't Fix"

Patches for CSSTidy are welcome & will be accepted; but me fixing this library is out of the scope of this project. The github project would be the correct place for fixes to CSSTidy.

Here is the patch that was committed.

AttachmentSize
advagg-1120266-22.patch 490 bytes