Javascript: selfmade methods not working correctly

Question

I tried to figure this out for some days now, I tried to use my own object to sort of replace the global object to reduce problems with other scripts (userscripts, chrome extensions... that kind of stuff). However I can't get things to work for some reason. I tried some debugging with JSLint, the developer tools included in Google Chrome, Firebug and the integrated schript debugger in IE8 but there is no error that explains why it doesn't work at all in any browser I tried.

I tried IE 8, Google Chrome 10.0.612.3 dev, Firefox 3.6.13, Safari 5.0.3 and Opera 11.

So... here is the code:

HTML:

<!DOCTYPE HTML>
<html manifest="c.manifest">
<head>
  <meta http-equiv="X-UA-Compatible" content="IE=edge,chrome=1">
  <meta charset="utf-8">

  <!--[if IE]>
     <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/chrome-frame/1/CFInstall.min.js"></script>
     <script src="https://ie7-js.googlecode.com/svn/version/2.1(beta4)/IE9.js">IE7_PNG_SUFFIX=".png";</script>
  <![endif]-->
  <!--[if lt IE 9]>
     <script src="js/lib/excanvas.js"></script>
     <script src="https://html5shiv.googlecode.com/svn/trunk/html5.js"></script>
  <![endif]-->

  <script src="js/data.js"></script>
</head>
<body>

<div id="controls">
     <button onclick="MYOBJECTis.next()">Next</button>
</div>
<div id="textfield"></div>

<canvas id="game"></canvas>

</body>
</html>

Javascript:

var that = window, it = document, k = Math.floor;
var MYOBJECTis = {

     aresizer: function(){
     // This method looks like it doesn't work.
     // It should automatically resize all the div elements and the body.
     // JSLint: no error (execpt for "'window' is not defined." which is normal since
     // JSLint does nor recognize references to the global object like "window" or "self"
     // even if you assume a browser)
     // Firebug: no error
     // Chrome dev tools: no error
     // IE8: no error
         "use strict";
     var screenWidth, screenHeight;
     if(window.innerWidth) {
         screenWidth = window.innerWidth;
        }
     else {
         if(document.documentElement && document.documentElement.clientWidth) {
            screenWidth = document.documentElement.clientWidth;
            }
         else {
            if(document.body) {
                screenWidth = document.body.clientWidth;
            }
        }
    }
     if(window.innerHeight) {
         screenHeight = window.innerHeight;
        }
     else {
         if(document.documentElement && document.documentElement.clientHeight) {
            screenHeight = document.documentElement.clientHeight;
            }
         else {
             if(document.body) {
                screenHeight = document.body.clientHeight;
            }
        }
    }
     (function() {
     for(var b = 0, c = it.getElementsByTagName("div");b < c.length;b++) {
         c[b].style.width = k(c.offsetWidth) / 100 * k(screenWidth);
         c[b].style.height = k(c.offsetHight) / 100 * k(screenHeight);
            }
        }());
     (function() {
     var b = it.getElementsByTagName("body");
     b.width = screenWidth;
     b.height = screenHeight;
        }());
    },


     next: function(){
     // This method looks like it doesn't work.
     // It should change the text inside a div element
     // JSLint: no error (execpt for "'window' is not defined.")
     // Firebug: no error
     // Chrome dev tools: no error
     // IE8: not implemented (starting at the var statement below)
         "use strict";
         var b = it.getElementById("textfield"), a = [], c;
         switch(c !== typeof Number){
            case true:
                 a[1] = ["HI"];
                 c = 0;
                 break;
            case false:
                 b.innerHtml = a[c];
                 c+=1;
                 break;
            default:
             return Error; 
            }
        }
    };
// auto events
(function(){
     "use strict";
     that.onresize = MYOBJECTis.aresizer();
    }());

If anyone can help me out with this I would very much appreciate it.

EDIT: To answer the question what's not working I can just say that no method I showed here is working at all and I don't know the cause of the problem. I also tried to clean up some of the code that has most likely nothing to do with it. Additional information is in the comments inside the code.

EDIT2: I updated the code and the comments inside it to what it looks like now but I still can't get it to work. I also got a new error on the next method from IE8.

Instead of just dumping your code, could you tell us _what_ doesnt work, what errors you get? Also, try to only include relevant code. — TJHeuvel, Jan 04 '11 at 13:45
@TJHeuvel, I disagree, inside the code is commented what should happen and where its going wrong, having the full code is very useful to recreate the problem and since its not that much of code i see no problem with it. — red-X, Jan 04 '11 at 13:52
I edited it a bit but all I can say is that if I want to use one of the methods inside my object they don't work. What they are supposed to do is provided in the comments inside the code. — hdr, Jan 04 '11 at 13:55
May I point out that it's good code to define type in all file imports. Also, there's something off about your structure.. — Mantar, Jan 04 '11 at 13:57

nedk · Accepted Answer · 2011-01-04T17:44:41.490

1

The main problem seems to be inside your loop:

c = it.getElementsByTagName("div");

returns an array of DOM Elements; and you're trying to apply styles on the entire array:

c.style.width

when you should be doing

c[b].style.width

In the next function your default case will never get executed because the condition will always be wither true or false and in both cases you're preventing the switch from moving down to the next case by using either a break or return - the default will only execute if there is no matching case defined or if the case above it doesn't break

Using document.getElementsByTagName returns an Array of Elements, hence the getElements.

You cannot apply styles to an Array of Elements, only to an individual Element, therefore you need to indicate the index value to the Element you're wish to modify, like so:

c[b].style.width = k(c[b].offsetWidth) / 100 * k(screenWidth) + "px";
c[b].style.height = k(c[b].offsetHight) / 100 * k(screenHeight) + "px";

and

var b = it.getElementsByTagName("body");
b[0].style.width = screenWidth + "px";
b[0].style.height = screenHeight + "px";

edited Jan 04 '11 at 17:44

answered Jan 04 '11 at 13:54

nedk

673
3
8

Furthermore, I try to avoid using strict mode, Pascal Martin gives a pretty good explanation here http://stackoverflow.com/questions/1335851/what-does-use-strict-do-in-javascript-and-what-is-the-reasoning-behind-it/1335881#1335881 – nedk Jan 04 '11 at 13:58
That sounds logical but for some reason even if I say c[b] it doesn't work. (I combined it with the suggestion of Andrey which shouldn't make a difference but still... same result). Concerning the break in the switch I always get an error from JSLint telling me that I need that break. Also I think it should only break if the case is true or am I wrong there? – hdr Jan 04 '11 at 14:29
A little addition: sorry about that... I just realised that what you say about my default is correct... stupid that I overlooked this... but still I am not sure about what you said about the break since I get this error from JSLint. – hdr Jan 04 '11 at 14:40
Oh and another thing I forgot: I also tied to remove the "use strict"; but that has no effect... – hdr Jan 04 '11 at 14:46
1. Your auto events need to be executed on load because the DOM elements need to be set up before you can do anything to them (see: http://www.w3schools.com/jsref/event_body_onload.asp) 2. You need to define units when you're setting style width, height, and position values. Simple appending `+ "px"` to the value fixes this. 3. Make sure you've got all the cases where you're not referencing an `Element` in an array i.e. `c[b]` and `b[0]` and when you're setting a `width` remember that `width` is part of the `style` property – nedk Jan 04 '11 at 15:38
You are right... it needs to be executed as an onload event of the body element, it makes the IE error go away. I also added + "px" but that however doesn't change much even with you being right about this. What I don't really get is what you mean with cases where I don't reference an element. If I say b = 0 shouldn't there be a case where I get just c[b] and if I define b = 0 and then directly say b+=1 or b++ shouldn't b be always be defined and at least 1 and referencing and element or do I have a big misconception here? – hdr Jan 04 '11 at 17:27
I have edited my answer to include code examples, compare them carefully with what you have and you will see what I mean by referencing an Element in the array. Also, be careful when using document.getElementById and document.getElementsByTagName; the former returns the actual Element object which you can modify, the latter returns an Array of Element objects. – nedk Jan 04 '11 at 17:49

score 0 · Answer 2 · answered Jan 04 '11 at 13:50

0

As a side note, you can use "window" instead of "that" - it's globally accessible.

I would use following to get client width and be compatible across the board:

if (window.innerWidth)
{
screenWidth=window.innerWidth;
}
else if (document.documentElement && document.documentElement.clientWidth)
{
screenWidth=document.documentElement.clientWidth;
}   
else if (document.body)
{
screenWidth=document.body.clientWidth;
}

answered Jan 04 '11 at 13:50

Andrey

20,487
26
108
176

Thanks for that. That looks like a good idea even though it doesn't really adress the root of my problem. :) – hdr Jan 04 '11 at 14:31

score 0 · Answer 3 · answered Jan 04 '11 at 14:17

0

I can see some very strange things in the next method.

You assign it.getElementById("textfield") to b but b is never used.

You declare c and then checks if c !== typeof Number which will always be tru as you have not set c yet.

I think next maybe should use variables created on the MYOBJECTis level instead of local variables.

answered Jan 04 '11 at 14:17

David Mårtensson

7,550
4
31
47

I do use b. The part you want to have a look at is b.innerHtml. Also my idea behind the c !== typeof Number is that if I call the the method for the first time it should set the value of c wich then becomes a number and if I call the method again its type is number and then it should go on with the other case. – hdr Jan 04 '11 at 14:38

score 0 · Answer 4 · answered Jan 04 '11 at 15:52

0

You are assigning an event handler. You just have to stick a function there, but not call it. So, this line:

 that.onresize = MYOBJECTis.aresizer();

Should be

 that.onresize = MYOBJECTis.aresizer;

Or

 that.onresize = function() {
     MYOBJECTis.aresizer();
 };

if you want to preserve the this in aresizer.

answered Jan 04 '11 at 15:52

Thai

10,746
2
45
57

thanks! that and the suggestion from nedk fix at least the new error I get from IE. – hdr Jan 04 '11 at 17:00

Javascript: selfmade methods not working correctly

4 Answers4