IE 8, what else. It seems to cause problems, it consumes the first line of the CSS for the .eot font but then it goes on and processes the second src line as well. The problem is that it doesn't really process it but tries to HTTP GET all three fonts at the same time, resulting in a 404, obviously. Things go unnoticed in a usual website but coupled with Ubercart, these 404 events cause problems (UC starts a new checkout session). Although this is clearly an IE 8 bug, there is no other solution than to work around it, I'm afraid...

Follow-up: the problem and a single-CSS workaround is described in detail at: http://readableweb.com/mo-bulletproofer-font-face-css-syntax/

CommentFileSizeAuthor
#11 local_fonts-7.2-iefix-1229786-7.patch3.56 KBBitvark
#12 local_fonts-6.2-iefix--1229786-7-D6.patch3.74 KBBitvark
#7 local_fonts-6.2-iefix--1229786-7.patch3.74 KBBitvark
#7 local_fonts-7.2-iefix-1229786-7.patch3.62 KBBitvark

Comments

djg_tram’s picture

djg_tram commented
Category: feature » bug

Here is the modified code in local_fonts_add_fontface_submit():

if ($format == 'eot') {
  // EOT goes on its own line and doesn't take a format.
  // Because IE is a "special needs" browser.
  $src_ie .= 'url("' . $file->filename . '")';
} // if

...

$generated_css = "@font-face { /* for IE */\n";
$generated_css .= "  font-family: '". $font_family ."';\n";
$generated_css .= '  src: '. $src_ie .";\n";
$generated_css .= '  font-style: '. $font_style .";\n";
$generated_css .= '  font-weight: '. $font_weight .";\n";
$generated_css .= "}\n";

$generated_css .= "@font-face { /* for non-IE */\n";
$generated_css .= "  font-family: '". $font_family ."';\n";
$generated_css .= '  src: url(http://:/) format("No-IE-404"), '. implode(', ', $combined_sources) .";\n";
$generated_css .= '  font-style: '. $font_style .";\n";
$generated_css .= '  font-weight: '. $font_weight .";\n";
$generated_css .= "}\n";

This separates the IE-specific font from the rest.

sreynen’s picture

sreynen commented
Title: Send specific CSS to IE 8 » Local font option to use IE-friendly syntax
Category: bug » feature

This isn't really a bug in the module; it's a feature request. There's a bug in IE, and it sounds like there's also a bug in Ubercart. It seems pretty crazy to me that Ubercart would start a new checkout session based on a 404. If that's happening, you should definitely open an issue under the Ubercart project. Also, I'm not even clear on why those requests would be 404. Those files are there, aren't they? I thought the IE bug was just that it downloads files it can't use, not that it can't download the files.

As far as the CSS syntax, the syntax you linked is pretty old. There have been a few revisions since then, one of which is currently being output. I described here why I chose to use a version of the syntax that doesn't rely on CSS hacks to work around IE bugs:

http://drupal.org/node/930066#comment-4122330

That said, if someone wants to do a patch that makes this an option, I'll commit it. Or if someone at Microsoft wants to pay me, I'll write the patch myself. But I'm not very inclined to spend any more of my free time than I already do dealing with IE bugs.

sreynen’s picture

sreynen commented
Version: 6.x-2.8 » 7.x-2.x-dev

If anyone wants to do this, it should happen first in the 7.x-2.x branch.

djg_tram’s picture

djg_tram commented

It's a bug in IE all right but not in Ubercart, there are security reasons quoted behind this behavior. No, the problem with IE is that its HTTP requests are malformed and will not download the fonts. I do understand your frustration about supporting IE, I feel the same way, yet we have to... :-) That modification, even if not rolled into a patch, seems to work for me all right.

When I move to D7 (pretty much depends on Ubercart going past beta stage on D7), I'll re-check the issue and if there will be anything left to patch in this area, I will.

sreynen’s picture

sreynen commented

To be clear, any patch here needs to make this syntax optional, so when the hack required to get IE to work ends up breaking behavior in some standards-compliant browser, users can just turn it off rather than needing to update the code again.

sreynen’s picture

sreynen commented
Component: Code » Code (general)
Priority: Major » Normal
Status: Active » Postponed

Marking this as postponed until someone submits a patch.

Bitvark’s picture

Bitvark commented
Status: Postponed » Needs review
StatusFileSize
new local_fonts-7.2-iefix-1229786-7.patch3.62 KB
new local_fonts-6.2-iefix--1229786-7.patch3.74 KB

Hello,
facing same issue, I've developed two patch: one for drupal 6, one for drupal 7.

They provide a checkbox before importing a local font to choose IE-Friendly Syntax.

I've used Fontspring Extra Bulletproofiness Syntax (February 21, 2011):
http://www.fontspring.com/blog/further-hardening-of-the-bulletproof-syntax

Patches have to be applied to local_fonts.module in the fontyourface/modules/local_fonts folder.

Hope it'll be useful.

bye

Status: Needs review » Needs work

The last submitted patch, local_fonts-7.2-iefix-1229786-7.patch, failed testing.

Bitvark’s picture

Bitvark commented
Status: Needs work » Needs review

#7: local_fonts-7.2-iefix-1229786-7.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, local_fonts-7.2-iefix-1229786-7.patch, failed testing.

Bitvark’s picture

Bitvark commented
Status: Needs review » Needs work
StatusFileSize
new local_fonts-7.2-iefix-1229786-7.patch3.56 KB

Hello,
I've had some problem with the testbot for the sub-folder... I'm sorry for this
Here is the right D7 one.

Bye

Bitvark’s picture

Bitvark commented
Status: Needs work » Needs review
StatusFileSize
new local_fonts-6.2-iefix--1229786-7-D6.patch3.74 KB

And here is the D6 patch.

sreynen’s picture

sreynen commented
Status: Needs work » Needs review

Thanks Bitvark. I'll try to review these soon.

sreynen’s picture

sreynen commented
Status: Needs review » Fixed

The latest dev uses a more IE-friendly syntax (which is also more Android-friendly), so I think this should no longer be an issue.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Follow-up