How did a doctype break my JavaScript?

Question

I made a menu, and just now I added

<!DOCTYPE html>

to the document, and it ceases to function. However the code is still running, will still print to console on mouseover etc. I tried other doctypes and they break it too, it just stays static.

What is it about a doctype that could cause JavaScript to not work?

JS:

var total_width = "960";
var slide_width = "182";
var slide_margin = "10";
var expand_width = "374";
var icon_width = "32";
var icon_margin = "15";
var slide_number = "5";

function slideid(i) {
    var m = document.getElementById("slide"+i);
    var l = document.getElementById("slideimg"+i);
    var k = document.getElementById("slidetext"+i);
    var j = document.getElementById("slidediv"+i);
    return [m, l, k, j]
}

function compat() {
    if (window.opera) {
        for (var i=1;i<6;i++) {
            s = slideid(i);
            s[3].style.position="relative";
            s[3].style.bottom="46px";
        }
    }
    if (document.all && !(window.opera)) {
        for (var i=1;i<6;i++) {
            s = slideid(i);
            s[0].style.height="60px";
            s[1].style.display="none";
            var iecontents = s[2].innerHTML;
            s[0].innerHTML=iecontents;
            s[0].style.fontFamily="'astonishedregular',Impact,serif";
            s[0].style.fontSize="40px";
            s[0].style.color="#fff";
            s[0].style.textDecoration="none";
            s[0].style.padding="10px auto";
        }
    }
}

function expand(x) {
    if (document.all && !(window.opera)) {
        return
    }
    var shrink = new Array();
    for (var i=1;i<6;i++) {
        if (i === x) {
            var expander = i;
        }
        else {
            var d = shrink.length;
            shrink[d] = i;
        }
    }
    for (var i=0;i<4;i++) {
        s = slideid(shrink[i]);
        var slide_shrink = ((total_width-(5*slide_margin))-expand_width)/(slide_number-1);
        s[1].style.marginLeft=(slide_shrink-icon_width)/2;
        s[0].style.width=slide_shrink;
        s[2].style.display="none";
        s[3].style.width="0"
    }
    s = slideid(expander);
    s[1].style.marginLeft=icon_margin;
    s[0].style.width=expand_width;
    s[2].style.display="inline-block";
    s[3].style.width=expand_width-icon_width-icon_margin-7;
}

function shrink() {
    if (document.all && !(window.opera)) {
        return
    }
    for (var i=1;i<6;i++) {
        s = slideid(i);
        s[1].style.marginLeft=(slide_width-icon_width)/2;
        s[0].style.width=slide_width;
        s[2].style.display="none";
        s[3].style.width="0";
    }
}

And HTML:

<!DOCTYPE html>
<head>
    <link rel="stylesheet" href="style.css" />
    <script src="menu.js"></script>
</head>
<body onload="compat()">

<div id="Menu" onMouseout="shrink()">
    <a href="#" class="slide" id="slide1" onMouseOver="expand(1)">
        <img id="slideimg1" src="home.png" />
        <div id="slidediv1">
            <h2 id="slidetext1">Home</h2>
        </div>
    </a>
    <a href="#" class="slide" id="slide2" onMouseOver="expand(2)">
        <img id="slideimg2" src="sound.png" />
        <div id="slidediv2">
            <h2 id="slidetext2">Music</h2>
        </div>
    </a>
    <a href="#" class="slide" id="slide3" onMouseOver="expand(3)">
        <img id="slideimg3" src="blog.png" />
        <div id="slidediv3">
            <h2 id="slidetext3">Blog</h2>
        </div>
    </a>
    <a href="#" class="slide" id="slide4" onMouseOver="expand(4)">
        <img id="slideimg4" src="note.png" />
        <div id="slidediv4">
            <h2 id="slidetext4">Bio</h2>
        </div>
    </a>
    <a href="#" class="slide" id="slide5" onMouseOver="expand(5)">
        <img id="slideimg5" src="way.png" />
        <div id="slidediv5">
            <h2 id="slidetext5">Links</h2>
        </div>
    </a>
</div>
</body>

This is so... nostalgic. A blast from the past. To the OP: instead of fixing this script, is it possible to choose one of the ten million other menu scripts which were written in XXI century? :) — Sergey, Dec 12 '12 at 10:04
@Sergey I know there are many others, I just like making my own things. Think of it like a hobby, and a good path to learning things. — Liam Bigelow, Dec 12 '12 at 10:09
@LiamBigelow: I just tried to point out that the code does not follow current best practices, even remotely. It looks like it was written in year 1999. — Sergey, Dec 12 '12 at 10:39
@Sergey Oh, sorry I misunderstood. I know that, I think this was my second thing ever done in javascript, and learning things often follow very bad practices. I was about to fix it up when I broke it. — Liam Bigelow, Dec 12 '12 at 10:56
@Sergey Out of curiosity, what are the worst practices you see? — Liam Bigelow, Dec 12 '12 at 10:57
Ahh, this deserves a separate question, possibly on http://codereview.stackexchange.com/. Just to name a few: leaking globals, missing semicolons; unnecessary browser detection; `new Array()` syntax, the whole obtrusive onMouseOver thing... As a learning exercise visit http://jslint.com and tweak the script until it passes without errors - currently it stops at line 19 because of too many errors. — Sergey, Dec 12 '12 at 11:23
@Sergey Yep I understand. Those are all things I'm fixing. Thanks — Liam Bigelow, Dec 12 '12 at 11:31

score 11 · Accepted Answer · edited Aug 06 '17 at 07:13

You need to add "px" to marginLeft and width. Setting CSS style which requires units will not work without unit. And you are missing <html></html> tags.

function expand(x) {
    if (document.all && !(window.opera)) {
        return
    }
    var shrink = new Array();
    for (var i=1;i<6;i++){
        if (i === x) {
            var expander = i;
        }
        else {
            var d = shrink.length;
            shrink[d] = i;
        }
    }
    for (var i=0;i<4;i++){
        s = slideid(shrink[i]);
        var slide_shrink = ((total_width-(5*slide_margin)) - expand_width) / (slide_number-1);
        s[1].style.marginLeft=(slide_shrink-icon_width)/2 +"px";
        s[0].style.width=slide_shrink +"px";
        s[2].style.display="none";
        s[3].style.width="0"
    }
    s = slideid(expander);
    s[1].style.marginLeft=icon_margin +"px";
    s[0].style.width=expand_width + "px";
    s[2].style.display="inline-block";
    s[3].style.width= (expand_width-icon_width-icon_margin-7) +"px";
}

function shrink() {
    if (document.all && !(window.opera)) {
        return
    }
    for (var i=1;i<6;i++){
        s = slideid(i);
        s[1].style.marginLeft=(slide_width-icon_width)/2 +"px";
        s[0].style.width=slide_width + "px";
        s[2].style.display="none";
        s[3].style.width="0";
    }
}

And this fixed it, slides nicely now. A few things have been knocked out of line, but that's what I get for not starting with a doctype. Thanks. — Liam Bigelow, Dec 12 '12 at 10:08

score 2 · Answer 2 · edited May 23 '17 at 11:48

2

document.all is a Microsoft thing, not covered by a standard (more in this other Q/A). Remove all use of it and use document.getElementById as necessary instead.

My guess is that if you look in the JavaScript console, you'll see errors related to document.all not existing, because while whatever browser you're testing on supported it in quirks mode, when you switched it to standards mode (by adding a doctype), it stopped supporting it.

edited May 23 '17 at 11:48

Community

1
1

answered Dec 12 '12 at 10:03

T.J. Crowder

1,031,962
187
1,923
1,875

I see no errors in console from document.all, right now that's just my crude way of checking if they are on IE and not performing the animation if so, I didn't think that would break the rest of the code – Liam Bigelow Dec 12 '12 at 10:05

score 1 · Answer 3 · answered Dec 12 '12 at 10:04

1

Adding the HTML5 DOCTYPE (<!DOCTYPE html>) puts browsers into "Standards mode". Without it, the browser will be in "Quirks mode", which is essentially an IE5/legacy compatibility mode. MDN has more information on the subject, and a list of differences in behaviour in Firefox.

The biggest problem with your script is the use of document.all, which is a non-standard Microsoft thing. You'll want to use standard DOM stuff like document.getElementById.

answered Dec 12 '12 at 10:04

Olly Hodgson

15,076
3
40
60

I'm not using document.all for getting elements, just a crude method of browser detection, since the sliding does not play well in IE. – Liam Bigelow Dec 12 '12 at 10:11

score 0 · Answer 4 · answered Dec 12 '12 at 10:04

include your count script before closing body tag and dont forget to add closing tag of html

<!DOCTYPE html>
<head>
<link rel="stylesheet" href="style.css" />
<script src="menu.js"></script>
</head>
<body onload="compat()">

<div id="Menu" onMouseout="shrink()">
<a href="#" class="slide" id="slide1" onMouseOver="expand(1)">
  <img id="slideimg1" src="home.png" />
<div id="slidediv1">
    <h2 id="slidetext1">Home</h2>
</div>
</a>
<a href="#" class="slide" id="slide2" onMouseOver="expand(2)">
<img id="slideimg2" src="sound.png" />
<div id="slidediv2">
    <h2 id="slidetext2">Music</h2>
</div>
</a>
<a href="#" class="slide" id="slide3" onMouseOver="expand(3)">
<img id="slideimg3" src="blog.png" />
<div id="slidediv3">
    <h2 id="slidetext3">Blog</h2>
</div>
</a>
<a href="#" class="slide" id="slide4" onMouseOver="expand(4)">
<img id="slideimg4" src="note.png" />
<div id="slidediv4">
    <h2 id="slidetext4">Bio</h2>
</div>
</a>
<a href="#" class="slide" id="slide5" onMouseOver="expand(5)">
<img id="slideimg5" src="way.png" />
<div id="slidediv5">
    <h2 id="slidetext5">Links</h2>
</div>
</a>
</div>
</body>
</html>

How did a doctype break my JavaScript?

4 Answers4