Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IE8 support discussion #9

Closed
mathiasbynens opened this issue Jul 31, 2012 · 30 comments
Closed

IE8 support discussion #9

mathiasbynens opened this issue Jul 31, 2012 · 30 comments

Comments

@mathiasbynens
Copy link
Contributor

I understand a decision has been made not to support these browsers. Your blog post mentioned nothing would break in IE8, though — would you be open to updating Prism.js so it doesn’t throw errors in IE6?

(I quickly “tested” this by opening prismjs.com in IE6, so perhaps the issue is not in Prism.js at all — but either way it would be nice to have this mentioned on the home page.)

@LeaVerou
Copy link
Member

Prism itself shouldn't throw any errors on IE6. It's the page that does.

@mathiasbynens
Copy link
Contributor Author

Oh, super!

How about making this more clear in the documentation? Something like:

While syntax highlighting is not supported in IE6–IE8, Prism.js will not throw any errors in those browsers.

@LeaVerou
Copy link
Member

Oh good point. I also want to look into the possibility of writing a plugin that adds IE8 support. If that's possible, it would be perfect.

@mathiasbynens
Copy link
Contributor Author

A plugin that adds IE6–8 support would be perfect++ :)

@LeaVerou
Copy link
Member

Notice that I said IE8 support. Who bothers with IE6-7 anymore?

@mathiasbynens
Copy link
Contributor Author

I did notice you said IE8 only, hence my previous comment: “a plugin that adds IE6–8 support would be even better”. Sorry for the confusion.

Who bothers with IE6-7 anymore?

I do for some client projects, and for pretty much all browser-based open-source projects I release.

If I release a script that works on IE6 and I want to use a syntax highlighter for its documentation, I wouldn’t use one that only works in modern browsers — I would want the documentation to be highlighted in all browsers the script itself supports.

Of course, not all of Prism would have to support IE6+. E.g. instead of calling highlightAll, end users could avoid the querySelectorAll calls by fetching the elements with e.g. jQuery and simply calling highlightElement on each of them instead.

@LeaVerou
Copy link
Member

Oh, that's a good idea. Will look into it.

@LeaVerou
Copy link
Member

Hi Mathias,

I started work on this, here: http://prismjs.com/plugins/ie8/
I'm not sure how to solve the line break bug. It's not as simple as I thought. Apparently, it's not just that innerHTML discards line breaks, it's also that innerText silently discards a few (seemingly at random) when getting. Same with innerHTML. It seems there's no way to read those line breaks in the first place!
I guess I'll need to have a look tomorrow at how other highlighters solved this.

@mathiasbynens
Copy link
Contributor Author

/cc @jdalton

@slevithan
Copy link

I solved it in RegexPal by using outerHTML.

This comment is in my circa 1997 RegexPal source code:

/* outerHTML is used to work around the fact that IE applies text normalization
when using innerHTML, which can cause problems with whitespace, etc. Note that
even this approach doesn't work with some elements such as <div>. However, it
mostly works with <pre> elements, at least. */

And here's my old code for this:

function replaceOuterHtml (el, html) {
    replaceHtml(el, "");
    if (el.outerHTML) { // If IE
        var id = el.id,
            className = el.className,
            nodeName = el.nodeName;
        el.outerHTML = "<" + nodeName + " id=\"" + id + "\" class=\"" + className + "\">" + html + "</" + nodeName + ">";
        el = $(id); // Reassign, since we just overwrote the element in the DOM
    } else {
        el.innerHTML = html;
    }
    return el;
};

The replaceHtml function takes a node out of the DOM (for performance reasons), updates its innerHTML, then puts it back. Since replaceOuterHtml kills the old node, it returns a reference to the new node so that variable references can be restored (el = replaceOuterHtml(el, html)).

@LeaVerou
Copy link
Member

Hi Steven,

I'm aware of the outerHTML workaround, I had used it too back in the day. However, here it seems that the problem is getting the line breaks.

Hmmm.

Unless outerHTML could help with that too. Will try this tomorrow!

@ccampbell
Copy link

@LeaVerou I realize we have competing projects, but I think prism looks nice. I have the same problem in https://github.com/ccampbell/rainbow and haven't had the time or energy to find a fix for it.

I was able to do some hackery to get it to somewhat work, but I ran into the same issue where some linebreaks were just missing completely.

You can look at the discussion here:
ccampbell/rainbow#69 (comment)

It's nice that most developers don't use these browsers so therefore the target audience for showing code samples is usually people using modern browsers.

@LeaVerou
Copy link
Member

LeaVerou commented Aug 3, 2012

Thank you @ccampbell! I quite like Rainbow too, kudos! Thanks a lot for the discussion, that will help very much!

However, I'm a bit too busy with my new job right now, so anything Prism-related will have to wait a bit.

@danielwertheim
Copy link

Just curious, is there anyone that has managed to make a plugin to support IE8?

@lukaszlenart
Copy link

I'm using Prism and calling it manually to apply highlighting to dynamically rendered html sections but under IE8 I get such error message:
2014-08-11_1250

I'm using such a CoffeeScript code

Prism?.highlightElement $('#element-id pre code')[0]

The issue happens when loading prism.js, do you have any idea how to solve this?

@apfelbox
Copy link
Contributor

Could you please use a non-minified version, so that the error location is more narrow?

@apfelbox
Copy link
Contributor

@danielwertheim there already is one: https://github.com/LeaVerou/prism/tree/gh-pages/plugins/ie8
although it is not linked in the plugins page or in the downloader on prismjs.com.

@lukaszlenart
Copy link

@apfelbox here you have
2014-08-12_0923

I think the problem is with WorkerGlobalScope and when I removed the whole highlighted code the previous issue is gone but new one appeared:
2014-08-12_0925

@apfelbox
Copy link
Contributor

Ok, addEventListener is in fact undefined in IE8, but I will need to take a closer look at the first example.

@LeaVerou
Copy link
Member

The reason the plugin is not listed anywhere in the website is that it’s not finished. It solves some issues with IE8 but not all, so I just gave up at some point as it wasn't worth that much effort for a dying browser.

Regarding your bug @lukaszlenart, the new issue is because the pile of shit called IE8 doesn’t even support addEventListener, but needs attachEvent instead. Also, it looks like this is code from a plugin you’re using, not Prism itself, which is what the IE8 plugin would target anyway, even if it was working.

Furthermore, I’d find it very strange if WorkerGlobalScope caused an issue in IE8, as that branch shouldn’t even be evaluated, since window Is defined there.

@lukaszlenart
Copy link

I'm not using the ie8 plugin, I have solved the problem with

  if (document.addEventListener) {
    document.addEventListener('hashchange', applyHash);
  }

@lukaszlenart
Copy link

(must test on other browsers)

@LeaVerou
Copy link
Member

That would just prevent the error, but it would also remove the functionality of changing the hash to highlight certain lines (which you may not want anyway) in IE8. I’d hardly call it a “solution”, more like sweeping under the rug.

@lukaszlenart
Copy link

highlighting doesn't work at all in IE8 ;-) I was wrongly assuming that Prism object is null/undefined under IE8 and base my logic on that - unfortunately it is a valid object so my Prism?.highlightElementdoesn't work as I expected.

@apfelbox
Copy link
Contributor

I will take a look at it - unfortunately not before I can use my other computer (currently running 10.10 and parallels doesn't work on it anymore (just like everything else doesn't work anymore, yay)).

@LeaVerou
Copy link
Member

Prism is not supposed to work in IE8. That would require too much code for an edge case. That was the whole point of developing a plugin for it.

However, IE8 support is definitely not a priority.

@lukaszlenart
Copy link

I don't care about supporting IE8 - just don't break anything in IE8 ;-)

@LeaVerou
Copy link
Member

Oh, that’s a different issue. Yes, nothing should break. If it does, we should fix it.

@lukaszlenart
Copy link

Right now I've implemented my changes and testing them, when new version of PrismJS will be released I will give it a try :-)

Pinpickle added a commit to Pinpickle/prism that referenced this issue Jan 12, 2015
@benplum
Copy link

benplum commented Mar 23, 2015

Lots of errors in IE8, starting with the 'self' reference at the start of prism-core.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants