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

Hard crash #17

Closed
pyrsmk opened this Issue Apr 28, 2011 · 37 comments

Comments

Projects
None yet
10 participants

pyrsmk commented Apr 28, 2011

Hi,

I've experienced a big crash on IE6-8 standalone versions with your last sources (04/19) ^^

Owner

scottjehl commented Apr 28, 2011

Hmm. Okay. Can you provide an example URL for debugging? I'm running latest on a project or two locally with no problems in IE. Thanks!

pyrsmk commented Apr 29, 2011

http://home.dreamysource.fr/dreamysource.fr

The site can be down (on night especially) because, as you see, it is hosted at home ^^

Maybe it crashes because I don't use correctly your library... The latest version I had didn't work (but did not crash at all, just did nothing...).

PS: the media query is at the bottom of the stylesheet
PS2: there's many comments without code, it comes from I compiled my sheets with Sass

mware commented Apr 29, 2011

I'm having the same issues. Hard crash in IE8, and no media query response in IE7. I'm using respond.js in a trial marriage of 320 and Up with Drupal 7. I eliminated scripts one by one until it became clear that respond.min.js (and also respond.src.js) was the culprit.

If I eliminate every bit of js on the page, I can at least see the static html. If I then enable only respond.min.js, IE8 crashes.

However, if I use the stand-alone 320 and Up demo, it works.

I know that drupal 7 has corrected namespace issues with different js libraries:
All code that expects to use jQuery as $ should be wrapped in an outer context like so.
(function ($) {
// All your code here
})(jQuery);
Would this impact the implementation of respond.js?

I'm developing the site on my machine and can't post it to a dev site, which makes troubleshooting more challenging. But does anyone have suggestions? Running Windows XP on VMWare. The MS error report is about 3 miles long, otherwise, I would paste the code here.

Any feedback would be greatly appreciated. Seems like a great plugin and the way for media queries to break through to the mainstream.
Thanks in advance.

pyrsmk commented Apr 29, 2011

The same for me. Without any media queries in my stylesheet, with just respond.js, IE6/7/8 crashes. It seems to load the page, and reload, and rereload until the crash.

Please note that I run IE with WinXP under VirtualBox ;)

Owner

scottjehl commented Apr 29, 2011

Sorry for the trouble, folks. Seems we're hitting an edge case here.

Can you drop this version in and tell me how it runs for you?
https://github.com/scottjehl/Respond/blob/0e35c3b9fc01be5f4ef07385a10da335c32f1f7e/respond.src.js

Thanks

pyrsmk commented Apr 29, 2011

No crash anymore, but no mq support too ^^

On the other side, I omit to tell you that your previous version didn't work for me too, that's why I'd get the 04/19 version... So, perhaps we can tell the bug is closed. And open another one? :D

mware commented Apr 29, 2011

Thanks for the quick reply. Dropped in the new version. Media queries now work in IE7, but IE8 still crashes.

pyrsmk commented Apr 30, 2011

Correcting my replies. I was affected too by issue 12. So, after drop the line-break put before the comment by compass it still crash everywhere (and I correct the fact that your anterior version doesn't work with me).

Owner

scottjehl commented May 3, 2011

I'm thinking there may be a conflict with @font-face in the parser. If you remove your font includes, do things work better? I'll look into the fix...

pyrsmk commented May 4, 2011

I've found what didn't work. There's no conflict with @font-face but I hadn't put a /mediaquery comment after a @media print from a base stylesheet... Then it matched from the @media print to the @media screen's close bracket.

Maybe @mware has another issue...

mware commented May 4, 2011

@scottjehl Bingo! stopped using @font-face via Google web fonts and IE8 revived. Well done.

Owner

scottjehl commented May 4, 2011

k. Sounds like my regexps are a little too greedy. BRB ;)

Owner

scottjehl commented May 4, 2011

Curious, how were you including the google fonts? Via link, @import, your own @font-face?

necolas commented May 4, 2011

@scottjehl Is @pyrsmk's issue with not including /mediaquery after an @media print block also involved in this? I haven't included the Respond comment after any blocks that don't need Respond, e.g., print. Can this cause problems?

mware commented May 4, 2011

linking to the google stylesheet with

then call font in a font stack.

mware commented May 4, 2011

sorry, code didn't come thru. using link tag, not @import or my own @font-face.

Owner

scottjehl commented May 4, 2011

@necolas BINGO! So yes, currently you must close all media blocks with the comment syntax, even if they're just a type like print. Otherwise, the matcher will keep going until it hits a closing query.

I'll try and fix it so it this isn't a requirement anymore.

That aside, the overhead is low: Respond basically groups all styles by type and injects separate style elements in the head per each type grouping (screen, all, print, projector, etc).

Anyway, I'll keep this open until this limitation is gone. Thanks for the help!

pyrsmk commented May 4, 2011

Sounds great :) It will fix several problems at once.

But, I don't understand why the overhead will be low since you can't use a regexp to remove the necessity of comments and, therefore, use a little parser?

Owner

scottjehl commented May 4, 2011

Oh I just meant that including the comment token so the print block would be parsed probably wouldn't cause a noticeable difference in response time. Anyway, I've got a fix coming that'll negate the need for comment tokens that follow statements you don't want/need to be translated, like @media print.

@scottjehl scottjehl closed this in c79e5d3 May 4, 2011

pyrsmk commented May 5, 2011

I wondered... Respond can handle this: @media screen and (min-width:768px), print ???

Owner

scottjehl commented May 5, 2011

yep. We're doing that internally on a project. You could also consider
@media all and (min-width:768px) {

...but yeah multiple queries are supported.

pyrsmk commented May 6, 2011

Good to know. Thanks for all your work :)

pyrsmk commented May 18, 2011

Actually, IE8 still crashes with the last Respond version, and randomly crashes with the parser version.

Same here - IE8 is still crashing. IE9 is looking awesome.

@ghost

ghost commented Sep 6, 2011

FWIW, I found that IE8 crashed when I called respond.min.js from the head of my HTML document (after the css calls). Once I moved the respond.min.js reference to the bottom of my document (which I should have done in the first place), it worked fine, no crash.

emb03 commented Nov 16, 2011

Hi,

This is not working for me in IE8. Did everyone give up on this?

dapen85 commented Jan 9, 2012

Still broken for me in IE8 using font face.

I have a simple demo page here. Using @font-face to load a local font, then included respond.min.js near the closing /body tag.

I get these errors in IE8:

Syntax error: respond.min.js Line:2 Char 1337

And of course, the media-query functionality is not working.

Thanks @scottjehl for your help looking at this.

Hopefully this is still open?

Seeing this issue as well.

When I include it in the head, instant IE8 hard crash. At the end of body, the page loads about 1/4 of the way, then a hard crash. Any updates?

Owner

scottjehl commented Feb 13, 2012

@andrewpeel I checked out that demo page but it looks like respond isn't included (it's in a comment though). Is that the demo I should be checking out?

@elysholladay can you make a demo page for me to debug? Thanks!

Can I email you a link privately?

There is a link to the respond version at the bottom - /respond.html

If not ill re-post

Thanks Scott

Owner

scottjehl commented Feb 13, 2012

Thanks Andrew. Would you mind updating that to version 1.1 and confirming that it's still a problem before I test it? Looks like 1.01pre is in there currently - lots of bug fixes since then :)

Thanks

Hey Scott - updated with the latest version. Doesn't crash now - nice. Also
doesn't work - unless my user error in implementation (most likely).

Cheers

emb03 commented Feb 13, 2012

New version works for me now in IE8 Thanks!

Hi Scott,

I may have the same problem but not sure. IE8 crashes with respond.js. Without, it works. Tried to "deactivate" the font-face and Google Web fonts but it does not change anything. Could you have a quick look at my website ?

http://peaxl.francischouquet.com/wordpress

Thanks :)

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