Support IE7 - Githubissues

ghost commented 10 years ago

You have a few javascript errors including trailing commas at the end of your arrays and the use of textContent property with regards to the DOM object (not supported in IE7/IE8).

leeoniya commented 10 years ago

thanks for the heads up. i pushed a couple commits to the lib and demo pages that hopefully fix the issues. i'm on Win8 and IE11's emulation modes are flaky, i got the errors initially after switching to IE7/8, but then they went away after a few toggles despite not having changed any code. this still persisted even after the browser was restarted. quite odd, but i dont feel like restarting my computer for the pleasure of debugging IE7/8.

let me know if the changes helped.

leeoniya commented 10 years ago

this may not work fully, i will push another commit which will still be broken because for some reason even some text nodes return an undefined .textContent in IE. see http://stackoverflow.com/a/1359822/973988

there's a polyfill for IE8, which i could not get to work http://eligrey.com/blog/post/textcontent-in-ie8

ghost commented 10 years ago

You're right. IE7/8 is does not support textContent at all. http://stackoverflow.com/questions/18326689/javascript-textcontent-is-not-working-in-ie8-or-ie7

the polyfill didn't work for me either but I was using IE7 to test it.

Your recent patch (as shown below) works on my IE7 browser.

hcell[ieLte9 ? "innerText" : "textContent"] = cfg.col_pre + i;

Thanks!

leeoniya commented 10 years ago

there are really several issues.

IE7/8 have .innerText that can be used instead of .textContent on DOM nodes only (not text nodes). .innerText doesnt always behave the same, but will probably suffice.
text nodes in IE7/8 are supposed to have .textContent but NOT .innerText, according to the SO post: "...means that you can "retrieve" textContent but not innerText from text nodes". the last commit tries to do this, rather than using the conditional to see which prop to read.
(2) above doesn't seem to work. it tells me .textContent is undefined on textNodes in IE during rendering. I don't know if this is a quirk in the IE7/8 emulation modes within IE11 or something else.

ghost commented 10 years ago

I just found another issue: ieLte9 is undefined because document.documentMode is undefined in IE7.

var ieLte9 = document.documentMode && document.documentMode < 9;

Also, 2) doesn't work for me either in IE7 running in a virtual machine (no quirks mode).

leeoniya commented 10 years ago

fixed the version detection. let me know if you figure out a way around 2).

ghost commented 10 years ago

I got around 2) by using nodeValue but I'm not sure if that's the best property to use.

leeoniya commented 10 years ago

i tried using .nodeValue as well. it doesnt trigger any errors, but the output's whitepace - at a minimum its line breaks - is completely messed up.

s1rd4v3 commented 10 years ago

so theres no fix for this one with IE8 and .textContent error?

leeoniya commented 10 years ago

unfortunately, every .textContent polyfill that i've found screws up line breaks and whitespace in some major way. i just tried out the recent https://github.com/shawnbot/aight/blob/master/js/element-properties.js and it didn't help the situation. if you can get the following simplified parser code to work the same way as ie9+, chrome and FF, in IE8 using .nodeValue, innerText or some other magical combo in place of .textContent, then we can move forward on this.

proper stringified output will show all leading/trailing whitespace (tabs, spaces, line breaks).

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <title>reMarked parser test</title>
</head>
<body>
    <p id="test">
        Lorem ipsum is my favorite ipsum
        it is really awesome and i like it
        <em>very much</em>, is my <strong>favorite</strong>,
        see?
    </p>

    <script>
        function parse(el) {
            var enode = {
                type: el.nodeName.toLowerCase(),
                kids: []
            };

            if (el.nodeName == "#text")
                enode.kids.push(el.textContent);
            else if (el.hasChildNodes()) {
                for (var i = 0; i < el.childNodes.length; i++) {
                    var el2 = el.childNodes[i];
                    enode.kids.push(parse(el2));
                }
            }

            return enode;
        }

        var el = document.getElementById("test");

        var tree = parse(el);

        console.log(JSON.stringify(tree));
    </script>
</body>
</html>

leeoniya commented 10 years ago

here's the raw version with tabs: https://gist.githubusercontent.com/leeoniya/570a7a12b0b1565585d8/raw/3f186c21e478668cc96125868a49ae8f9f0241b4/gistfile1.html

tukuna30 commented 9 years ago

Got a fix for IE8 text nodes using 'data' property. 'data' or 'nodeValue' to get value of text nodes on IE8. this.c = (this.e[textContProp] || this.e['data']).split(/^/gm); /* line:652 */

http://quirksmode.org/dom/core/

leeoniya commented 9 years ago

thanks for the tip.

this makes it feasible, but not completely trivial. i swapped it in and ran the /tests/gen_md/index.php and it started complaining in other places. looks like this property only exists for textNodes, but i use textContProp in a couple other places that are not text nodes.

i suspect that these additional places can be rewritten to emulate what .textContent does via an additional recursive function to loop through all sub-nodes, concatenating every textNode's data, but this will add some code and will need more testing. if you have the time and feel like it's worthwhile, please submit a pull request that has no errors and 0 diff in /tests/htm_md/reMarked after running the testcases. if i have time, i may take a stab at it myself, but no promises.

http://stackoverflow.com/questions/12286975/when-working-with-text-nodes-should-i-use-the-data-nodevalue-textcontent

leeoniya / reMarked.js

Support IE7 #31