Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Jade browser client .js does not work in IE7 or 8 #254

Closed
weixiyen opened this Issue · 30 comments

10 participants

@weixiyen

Are there any plans to support IE7 and IE8?

@tj
Owner
tj commented

I have no VM set up to test IE unfortunately so I'm not much of a help

@TooTallNate

Can you tell us what the error message you are seeing is? I'll probably help diagnose what's breaking.

A filename/linenumber would be best...

@weixiyen

I think have a fix and will provide a diff to jade.js shortly. It was a combination of 2 things

1) resolve/require didn't work correctly in IE

2) using stuff like forEach, inArray, string.trim(), etc

Still testing to see if the parsing is correct, so far looks to be ok.

@weixiyen weixiyen closed this
@weixiyen weixiyen reopened this
@guyht

There are quite a few libraries around that add missing functionality to older browsers (mainly IE7&8). They add functions like inArray, string.trim() etc...

It might be worth testing Jade with some of these libraries and then just make it a dependency for older browsers.

@tj
Owner
tj commented

each enough to avoid using trim() etc

@weixiyen

Just FYI, this is the code I am using currently that seems to work in IE7 and IE8.

http://173.255.240.43/js/jade.js?v=e4420831-0fb6-4523-a68f-1b9df637bec1

I'm not sure on next steps though on what I should do regarding this bug.

@weixiyen

It appears the following cases makes this work.

1) Using prototype.js
2) Extending the native Array prototype with a forEach function.

The code I posted in the previous comment does NOT work.

I'm not sure why this is, but it appears that without something in prototype.js, the HTML output by Jade will not include classnames, attributes, IDs, etc... yet it will include content inside div tags.

Any ideas?

@guyht

Could this be to do with the forEach function?

If there is no forEach function then Jade cannot loop through each of the attributes and add them to the divs...

@weixiyen

It depends on the native forEach function being there IN ADDITION TO something in prototype.js (which I've yet to identify exactly what).

Prototype.js provides Array.indexOf and Array.map, but it also provides something else that somehow makes the jade parse correctly. Otherwise jade will parse without classnames, ids, etc.

@tj
Owner
tj commented

it's not like we have to use forEach :p I can easily change that

@tauren

When I test running a precompiled template function in IE8, I get an error on the first line of code in the following. This code is within the rethrow function:

// Error context
var context = lines.slice(start, end).map(function(line, i){
  var curr = i + start + 1;
  return (curr == lineno ? '  > ' : '    ')
    + curr
    + '| '
    + line;
}).join('\n');

I'm guessing the problem is that Array.map doesn't exist in IE8. This could either be rewritten to not use map, or a requirement could be imposed to include es5-shim or something similar.

Note that I'm not loading or running jade in the client. I'm sending precompiled templates as raw javascript from the server. The templates were compiled on the server and saved out as a string compiled.toString(), then served to the client.

@guyht

I agree that this needs to be addressed.... especially for cases similar to @tauren.

IMO I dislike de-optimising code for all browsers just so it will work in IE, so would prefer to see a requirement that Array.map is implemented manually if not already supported.

Either state that a shim script is required, or, as part of the rendering process in Jade, implement the missing functionality as required.

@tj
Owner
tj commented

I'd rather stay away from shims, you'll otherwise have bloat from libs all adding conditional shims that they need, or if we specify that you just need to add one as a dep it's an extra annoyance, we might as well just use a for loop, there's already a lot of ugly code due to client-side stuff anyway

@guyht

My suggestion would have been to add it as an extra dependency as I hate the fact that I constantly have to write unoptimised code so it will be compatible with IE. I agree that this is an extra annoyance but it is also becoming more common among client side libraries.

In this case, replacing Array.map with a loop would be near trivial, but it also feels like defeat.

@tj
Owner
tj commented

using map etc isn't really optimizing, it's slower, not that speed is relevant in that chunk of code anyways, but I know what you mean, I really dislike catering to both environments.

@brandonbloom

Object.keys is also an issue.

If you are using underscore, you can add this shim (CoffeeScript):

unless Array.prototype.map?
  Array.prototype.map = (callback, context) -> _.map(@, callback, context)
unless Object.keys?
  Object.keys = _.keys

Or if you haven't drank the coffee/cool-aid yet:

if (Array.prototype.map == null) {
  Array.prototype.map = function(callback, context) {
    return _.map(this, callback, context);
  };
}
if (Object.keys == null) {
  Object.keys = _.keys;
}  

Like @tauren, I'm sending compiled templates down. I'm using Stitch:

stitch.compilers.jade = (module, filename) ->
  content = jade.compile fs.readFileSync(filename, 'utf8')
  module._compile "module.exports = #{content};", filename

A proper patch to avoid calls to these two methods would be nice. If someone would like to take a crack at it. I'm just gonna run with this shim for now.

@tj
Owner
tj commented

stitch requires you to write sync?

@masylum

what's the status of this? Would make sense to use mocha for tests and run them on different browsers?

@tj
Owner
tj commented

This issue has been inactive for over 2 months so I'm closing it. If you think it's still an issue re-open. - tjbot

@tj tj closed this
@paulyoung

I'm having the exact issue @weixiyen mentioned about missing attributes in IE.

I've tried adding missing functions for IE from this StackOverflow page but not getting anywhere.

Did anyone get this working?

@paulyoung

For what it's worth, I believe that this is the cause:

/**
 * Lame Array.isArray() polyfill for now.
 */

if (!Array.isArray) {
  Array.isArray = function(arr){
    return '[object Array]' == toString.call(arr);
  };
}

/**
 * Lame Object.keys() polyfill for now.
 */

if (!Object.keys) {
  Object.keys = function(obj){
    var arr = [];
    for (var key in obj) {
      if (obj.hasOwnProperty(key)) {
        arr.push(obj);
      }
    }
    return arr;
  } 
}

If I replace those polyfills with the following, everything seems to work:

if (!Array.isArray) {
  Array.isArray = function(arr) {
    return _.isArray(arr);
  };
}

if (!Object.keys) {
  Object.keys = function(obj) {
    return _.keys(obj);
  };
}
@paulyoung

I didn't determine if one or both or these are required.

@tj
Owner
tj commented

we should get rid of both polyfills anyway, a lib isn't the right place to add those, when I have a sec I'll remove those and use utils instead of Object.keys() etc

@tj tj reopened this
@paulyoung paulyoung referenced this issue in deedubs/require-jade
Open

Still carrying a bug from older jade version #2

@ForbesLindesay

Is this still an issue?

@paulyoung

We're still using the polyfills I mentioned above. I'm assuming it's still an issue since this is still open.

@ForbesLindesay

@paulyoung I've been going through and closing many of the old issues that turn out to have been fixed ages ago, I just wanted to confirm that this is still an issue and not just something nobody got round to closing.

@paulyoung

I believe this is still an issue as the "lame" polyfills still exist exactly as before.

@samccone

Yup I have encountered this as well. Should be a simple fix I shall look into it @ForbesLindesay

@ForbesLindesay

Meh, this is a non-goal, just add the necessary polyfills for any missing functionality using something like mondenizr. Force your users to update already.

@jeromew jeromew referenced this issue
Closed

Roadmap #1254

0 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.