why isn't this working for me

Discussion in 'JavaScript' started by saturn100, Nov 5, 2013.

  1. #1
    I havent recieved any info for my question yesterday so did some digging around and found this solution here

    http://jsfiddle.net/pxfunc/WWNnV/4/
    This will work for me and I tried to run it my self but had no luck
    Why is it running off jsfiddle and not for me

    My page is

    <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
    <html xmlns="http://www.w3.org/1999/xhtml">
    <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
    <title>Untitled Document</title>
    <script type="text/jscript">
    
    
    
    document.getElementById('upload').onchange = uploadOnChange;
       
    function uploadOnChange() {
        var filename = this.value;
        var lastIndex = filename.lastIndexOf("\\");
        if (lastIndex >= 0) {
            filename = filename.substring(lastIndex + 1);
        }
        document.getElementById('filename').value = filename;
    }
    </script>
    
    <style type="text/css">
    
    </style>
    </head>
    
    <body>
    <input id="upload" type="file" />
    <input id="filename" type="text" />
    </body>
    </html>
    
    HTML:
    I have tried it on and off a server in Chrome and firefox
     
    saturn100, Nov 5, 2013 IP
  2. FilmFiddler

    FilmFiddler Greenhorn

    Messages:
    54
    Likes Received:
    7
    Best Answers:
    0
    Trophy Points:
    23
    #2
    Move the entire script from the <head> to the body, just before the </body> tag. Scripts should generally be placed there, with few exceptions.
    As it stands, the 'upload' <input> does not exist on the page, at the time getElementById() is trying to reference it.
    A 'loose' (outside a function) call to getElementById() in the head is guaranteed to cause a script to fail.
     
    FilmFiddler, Nov 5, 2013 IP
    deathshadow likes this.
  3. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #3
    FilmFiddler has it right -- but to clarify the second statement, as the browser parses your markup elements are added to the DOM in the order they appear, and any SCRIPT tags are run BEFORE parsing of the DOM can continue -- so if you put your script before the element you are trying to target with methods like getElementById, it simply doesn't 'exist' on the DOM yet -- so that's where your code will fail.

    One nice side effect of that is that if you move the scripts to right before </body> they will run before the DOM is closed, even though the DOM is complete (all elements exist)... if all you are doing is attaching handlers, modifying/generating elements or adding classes to the page, you can do so here BEFORE CSS is applied to the document in most browsers. This prevents the "jump" into place most developers encounter when they wait for onload.

    ... which is part of why I've stopped using onload altogether.

    It's also a good point that ALL scripting should wait until right before </body>, and I would include in that all the various onevent functions like onchange or onsubmit; those really have no business in the markup in the first place and you're generally better off hooking the elements from the script instead of typing it out in your HTML.
     
    deathshadow, Nov 5, 2013 IP
  4. saturn100

    saturn100 Well-Known Member

    Messages:
    465
    Likes Received:
    0
    Best Answers:
    0
    Trophy Points:
    111
    #4
    Thanks
    I got it working and added features to it that I needed
    Really happy with my work and plan on giving it back to JSFiddle

    Question if instead of writing the JS into my page can I reference it via an external document
    If so what do I need to change
     
    saturn100, Nov 6, 2013 IP
  5. deathshadow

    deathshadow Acclaimed Member

    Messages:
    9,732
    Likes Received:
    1,999
    Best Answers:
    253
    Trophy Points:
    515
    #5
    It is actually PREFERRED to put it in another document, especially if it is being loaded across different pages, but also if the same visitor is visiting the same page more than once. Just take your code, put it in an external file -- let's call it 'library.js' for now.

    <script type="text/javascript" src="library.js"></script>

    Is all you need to include it on the page.

    I did notice some errors that might cause you headaches across browsers. "this" would refer to function this, not the target element's this -- it is best to parse that out of what's passed to the function as an event or via window.event depending on the browser. Also not all systems use backslashes, so you might want to clean backslash to forward slash before doing the indexOf... though I'd probably split and pop instead of indexof/substr. You could also simplify it down if you're not calling "onchange" via any other event...

    document.getElementById('upload').onchange = function(event) {
    	event = event || window.event;
    	document.getElementById('filename').value = (
    		event.target || event.srcElement
    	).value.replace(/\\/g,'/').split('/').pop();
    }
    Code (markup):
    Is how I'd code that same thing. 'this' is unreliable, we want the element that triggered the onchange so we get the event (or window.event for legacy IE), then we get the event's target (or srcElement for legacy IE) as where to grab the value from.
     
    deathshadow, Nov 6, 2013 IP