Bug Tracker

Modify

Ticket #13274 (closed bug: fixed)

Opened 15 months ago

Last modified 12 months ago

IE < 9: "'jquery' is undefined" w/ sourceMappingURL comment

Reported by: alexc223@… Owned by: alexc223@…
Priority: blocker Milestone: 1.9.1
Component: build Version: 1.9.0
Keywords: Cc: paul.irish
Blocking: Blocked by:

Description

When using MSIE < 9 and jQuery 1.9.0 I am getting the following warning/error:

'jquery' is undefined

This error is coming from line 4 in the minified versions, which is (a comment!):

//@ sourceMappingURL=jquery.min.map

Change History

comment:1 Changed 15 months ago by dmethvin

  • Owner set to alexc223@…
  • Status changed from new to pending

Can you provide a test page that we can use? I include *just* the minified 1.9.0 and it seems to work fine.

comment:2 Changed 15 months ago by chris@…

I can confirm this bug. You can see it live on  http://www.achievementstats.com/

comment:3 Changed 15 months ago by dmethvin

  • Priority changed from undecided to blocker
  • Status changed from pending to open
  • Component changed from unfiled to build
  • Milestone changed from None to 1.9.1

It definitely is happening on those sites, although I can't reproduce it with this simple jsfiddle:  http://jsfiddle.net/dmethvin/vvtJN/3/

If you get into the IE8 debugging console and type //@ sourceMappingURL=jquery.min.map you see the same error as above though, so it does seem real. We just need to figure out what conditions cause it.

comment:4 Changed 15 months ago by dmethvin

I am starting to feel sick...it's conditional code.

 http://www.javascriptkit.com/javatutors/conditionalcompile.shtml

If someone, anyone, uses //@cc_on before jQuery loads it looks like the sourceMappingURL line will cause an error in IE8.

 http://imgur.com/x2RL8KJ

Supposedly it can be turned off with @cc_off but that would have to be in OUR source file for defensive purposes and would affect following code.

comment:5 Changed 15 months ago by anonymous

This will affect *all versions* of IE, not just IE8. See  this fiddle in IE10 or below.

comment:6 follow-up: ↓ 20 Changed 15 months ago by paul.irish

Okay spent some time on this with Jake Archibald...

This is where WebKit parses the comment. It does NOT need to start on a newline, but there are some other specifics in the regex it's looking for:  https://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/inspector/ContentSearchUtils.cpp&exact_package=chromium&q=sourceMappingURL&type=cs&l=143

Two possible fixes...

jQuery-side fix..

Augment the current reference with flowerbox comments (on new lines).

/*
//@ sourceMappingURL=undeclaredIdentifier
*/

cc_on's will still work (and so will the sourcemapping), but IE won't try to parse this as a CC directive.

User-side fix

Since this is only repro'd when a page turns on conditional compilation before including jQuery, DONT DO THAT.

  1. Don't use @cc_on
  2. Include jQuery before one of your scripts turns on @cc_on
  3. stop using some super old legacy version of the html5shiv and update to the latest version
  4. Don't use @cc_on. Use this instead  https://gist.github.com/527683
  5. @cc_on dramatically slows down js evaluation. don't use it.

comment:7 Changed 15 months ago by alexc223@…

Yep, I can confirm that's the case - an old html5shiv was in use, which uses conditional compilation. Upgrading to the latest html5shiv (which no longer uses it), has resolved this for me.

comment:8 Changed 15 months ago by chris@…

AchStats.com uses this table sort script:  http://www.frequency-decoder.com/2006/09/16/unobtrusive-table-sort-script-revisited It has indeed conditional code, but that's nothing we could change. However loading jQuery before loading the sort script has resolved the issue! Thanks for the research. :)

comment:9 Changed 15 months ago by gibson042

As it turns out, we can fix this and even save a byte:  https://github.com/jquery/jquery/pull/1143

Whether we want to remains an open question.

comment:10 Changed 15 months ago by Richard Gibson

  • Status changed from open to closed
  • Resolution set to fixed

Fix #13274: Wrap sourceMap directive in multiline comments. Close gh-1143.

Changeset: ac93559eb9f18fcaec95dfdc97358b1b85bfe234

comment:11 Changed 15 months ago by Richard Gibson

Fix #13274: Wrap sourceMap directive in multiline comments. Close gh-1143. (cherry picked from commit ac93559eb9f18fcaec95dfdc97358b1b85bfe234)

Changeset: 487b703521e63188102c73e8ce6ce203d28f260b

comment:12 Changed 15 months ago by Matt Robenolt <matt@…>

I just noticed now in master that the //@ sourceMappingURL= indicator is now at the top of the document.

According to the version 3 spec for source maps, it explicitly says it should be at the end of the document. I'm not sure how these are implemented at the browser level, but could it cause any issues? Or should this idea be brought up to browser devs and have the spec loosened?

Spec document:  https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit#heading=h.txk3vdsh99hf

Compiled source from master:  https://gist.github.com/raw/43037ab16b9b22d1ceb0/gistfile1.js

comment:13 Changed 15 months ago by gibson042

I observe that particular statement to be in the "Conventions" section, and also that a strict reading would prohibit the trailing */ required by this ticket. Given that our current approach was specifically crafted to match browser implementation, who do we talk to about updating the spec?

comment:14 Changed 15 months ago by dmethvin

  • Cc paul.irish added

Probably good to pull in Paul Irish here. I am not at all a fan of the current "convention" of using a newline-terminated comment at the end of the file. It also makes more sense for the comment to be a the top of the file. But we should get all the parties involved to agree on what is within the spec.

While we're changing the spec it would be great to pick a sequence that wasn't already in use by @cc_on.

comment:15 Changed 15 months ago by Matt Robenolt <matt@…>

According to the source of both WebKit and FF, this will technically work. Both work by just looking for a comment that fits that pattern, which technically could be left open to the sourceMappingURL indicator being *anywhere* in the document.

(Slightly out of the scope of this discussion), but I think the beginning of the document makes a lot of sense for implementations other than a browser for one good reason:

It's possible to stream in bytes and detect a source map without loading the whole file into memory. If the source map could be detected early, you can disregard the rest and just fetch the source.

Granted, it's a small idea, but it makes sense to me. Or just strictly use the proposed SourceMap header, which seems the cleanest of them all.

comment:16 Changed 15 months ago by dmethvin

Using the HTTP header may seem like a good idea but isn't. Most web devs can change the content of a file with no problem but can't change the headers sent with the file without access to server configuration. It also won't work if the file is used locally.

comment:17 Changed 15 months ago by mattrobenolt

@dmethvin Good point. It is harder to maintain a header. So maybe just a preference for a header if possible. :)

comment:18 Changed 15 months ago by adam.dorsey@…

I still get an error in IE8, even when using the fix:

/* @ sourceMappingURL=undeclaredIdentifier */

comment:19 Changed 15 months ago by adam.dorsey@…

Sorry, here it is in a proper code block:

/*
//@ sourceMappingURL=undeclaredIdentifier
*/

comment:20 in reply to: ↑ 6 Changed 13 months ago by meelash

Paul, do you have any numbers/tests/explanation for why turning on cc slows down js? Is this in IE only (given that other browser should be ignoring it as a comment)?

Replying to paul.irish:

Okay spent some time on this with Jake Archibald...

This is where WebKit parses the comment. It does NOT need to start on a newline, but there are some other specifics in the regex it's looking for:  https://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/inspector/ContentSearchUtils.cpp&exact_package=chromium&q=sourceMappingURL&type=cs&l=143

Two possible fixes...

jQuery-side fix..

Augment the current reference with flowerbox comments (on new lines).

/*
//@ sourceMappingURL=undeclaredIdentifier
*/

cc_on's will still work (and so will the sourcemapping), but IE won't try to parse this as a CC directive.

User-side fix

Since this is only repro'd when a page turns on conditional compilation before including jQuery, DONT DO THAT.

  1. Don't use @cc_on
  2. Include jQuery before one of your scripts turns on @cc_on
  3. stop using some super old legacy version of the html5shiv and update to the latest version
  4. Don't use @cc_on. Use this instead  https://gist.github.com/527683
  5. @cc_on dramatically slows down js evaluation. don't use it.

comment:21 Changed 12 months ago by anonymous

Will this be fixed in a future version? I have web essentials installed which will automatically change my minified files back to the single line comment. The workaround of a multiline flower comment does not work with Web Essentials.

comment:22 follow-up: ↓ 23 Changed 12 months ago by dmethvin

On our end, the only option we have is to remove support for sourcemaps altogether.

Your tools should not be mangling that comment, I can't think of a reason why they would do that. If you are combining/(re)compressing files the comment becomes useless anyway because the map no longer represents the new file. Remove it or remove it not, there is no mangle.

comment:23 in reply to: ↑ 22 Changed 12 months ago by anonymous

Replying to dmethvin:

On our end, the only option we have is to remove support for sourcemaps altogether.

Your tools should not be mangling that comment, I can't think of a reason why they would do that. If you are combining/(re)compressing files the comment becomes useless anyway because the map no longer represents the new file. Remove it or remove it not, there is no mangle.

What I'm saying is that Web Essentials automatically creates the minfied file and map file when you save the unminified file. So if I save the unminified file, the minified file gets re-created with the single line comment.

comment:24 Changed 12 months ago by dmethvin

Well that's not jQuery's issue at all, Web Essentials is creating an incompatible file. You'll need to let the Web Essentials people know they're generating a file that causes errors in IE and/or convince the sourcemap folks to adopt a syntax that is compatible with IE's @cc_on. I haven't had much luck with the latter myself.

comment:25 Changed 12 months ago by m_gol

@dmethvin I've talked to Nick Fitzgerald at the Front Trends conference in Warsaw recently and he's open to changing the @ symbol, in his presentation about source maps he even directly said that it'll probably change. I'm optimistic about that.

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.