Building a better "show password"

One of the recent staff blog entries by James Edwards showed how to make a ‘show password’ checkbox that alternates between input[password] and input[text]

The only problem was it seemed a bit… heavy to me. 10k of javascript – you strip out the comments and it’s 5k… for a simple element swap? Didn’t sound right.

So I recoded it thus:
http://www.cutcodedown.com/codeExamples/showPasswordCheckbox/demo.html

Directory is unlocked for ease of access to the bits and pieces:
http://www.cutcodedown.com/codeExamples/showPasswordCheckbox

Which uses a good deal less javascript - to the tune of under 2.5k total depending on if you’re using IE (2.1k) or ROW (rest of world, 1.6k) though an extra handshake is wasted to do it.

But I figure why stop there - more eyes on target the better we can make this. Admittedly I used innerHTML, but I could give a flying … wolenza… about it’s “official” status given the train wreck doing that one line with the DOM would be (ten lines of code instead of two)…

But I’m thinking for other improvements perhaps run the script before </body> and have it auto-attach itself by a class trigger or ID prefix. Problem is the added elements are complex enough that you’d either resort to an extra container in the markup (which I’d like to avoid) or have a ton of DOM manipulating scripting to do “insertAfter” (nextSibling exists then insertBefore, nextSibling does not exist then parentNode.addChild – WHY is this not a standard function?!?)… I’d probably end up using the DOM to make the wrapping div, stick it in the markup then still innerHTML the rest of it. (I really feel there’s room for both methods).

But I’d be interested to see other people’s take on approaching this - smaller code would be better but I’m a bit rusty at .js and am just interested in other people’s opinions on the methods of coding what should be a fairly simple element change.

More eyes on target == better quality kill.

Oh, btw, script compression and single letter variables for major elements is NOT the answer to making it smaller. Neither is Jquery/prototype/mootools or any of that other nonsense that usually not only makes the code bigger, harder to read, more cryptic, and requires a 100k library to do it… because that’s “easier” – RIGHT.

Maybe it’s because I was weaned off assembly by Wirth family languages two and a half decades ago, but I’ve never understood this rabid dislike for “with” that has cropped up the past five or six years. I think it stems from people not understanding how namespaces work.

All objects get their own namespace table, and then there’s the global one. When objects are created as variables a pointer to the objects namespace is put on the global table. All “with” does is put another pointer to the namespace table on top of the stack… and it works like a stack, since whatever is on top is checked FIRST (which is also how function overrriding in interpreted languages often works - let garbage collection handle the dupes). Just occurred to me because it is checked first, it would actually execute faster than the full variable name (blessed be short circuit eval). Probably also why the overhead of the variable declaration approach isn’t as severe as it could be since it means you don’t have to parse as deep down the table. (since I’d figure document would be pretty close to the bottom – though with Chrome it’s zero since they seem to have a btree on their namespace)

The “confusion” in the article you linked to is typical of those who fail to grasp this:

Will ooo.eee.oo.ah_ah.ting.tang.walla.walla be modified? Or will the global variables bing and bang get clobbered? It is impossible to know for sure.

It’s entirely possible if you bother to understand how it works and PAY ATTENTION. The “with” set is evalled first, so if “ooo.eee.oo.ah_ah.ting.tang.walla.walla” contains “bing” and “bang” then those will be resolved first and short circuit exit. What the hell’s so confusing about that?!? :wink: Beats the hell out of a new namespace variable AND a memory allocation on the heap that is used in the ‘var’ example after. That quote off the YUI page is one of the dumbest questions I’ve seen in a while… Or maybe I’ve spent too much time working in Smalltalk, Pascal and Modula? (HATE C syntax languages – everything feels shoehorned in any-old-way.)

Which shows you don’t grasp the namespace or how with works – it’s a dropthrough. Just like switch/case – and even like how you used @cc_on with the double assignment. If none of the ‘with’ members in the current focus have that element in it’s namespace, the next parent on the global table has it’s table checked (or the next element on the global table). Simple.

It’s also how locals work - locals in most interpreters end up on the same stack as globals, they’re just popped off when the function ends. It’s where PHP is a bit different is each function has it’s own table, which is why you have to use the GLOBAL command to put pointers to global vars onto that local stack.

Really that YUI article is more FUD than fact if you grasp that it’s a drop-through… and other people’s inability to grasp how a common function works should not be a reason to not use it; if we did that, half the HTML specification would go unused… oh, WAIT! (LEGEND, LABEL, TH, SAMP, CODE, CAPTION, BLOCKQUOTE, FIELDSET, Proper numbered heading tag orders…) and you’d still have people using mySQL_ instead of PDO… oh, WAIT!!! (gah, “the mind boggles”)

Of course that said FUD article is on the YUI blog is hardly surprising given what a total train wreck their fat bloated library of trash is. How to flush websites down the crapper by ignoring the POINT of CSS so that one might as well be writing HTML 3.2, use endless goofy animated crap that does nothing but get in the way of getting at the page content, and of course trashing anything remotely resembling accessibility with fixed width containers, fixed height containers, and fixed metric fonts. (which sums up 99% of CSS and Script ‘libraries’ like mooTools and Jquery – one step removed from the WYSIWYG nimrods)

Taking web design advice from Yahoo is like getting technical advice from Forbes or financial advice from Popular Electronics. <Colbert>I for one am shocked – Does Yahoo still have customers?</Colbert><word>Google It</word>

But much of this could come from the difference in how I read code; I find color syntax highlighting a confusing acid-trip of gibberish that’s impossible to read; I find really really really long lines of code hard to read when they wrap (which is why javascript strings not being white-space neutral pisses me off and why I constantly take people to task for that nonsense in their CSS) – and for the life of me 99% of the time when people put extra spaces around operators like +,-,=, etc… I find it an illegible mess that makes me go “what, punctuation blind?”

Maybe I’ve just been at this **** for too long. Would explain why I can hand assemble Z80 and 8088 machine code one byte at a time from memory (I can even tell you the execution times in clocks), but cannot grasp how visual programming is even supposed to work! :smiley:

Quirksmode is a cute reference, but sometimes their opinions on many of the javascript elements seem incomplete. If you check ‘specified’ you will not end up copying anything except what the user declared in the markup… IE8 will also copy ‘value’ if it was modified, while IE7/earlier doesn’t change specified. The problems with attribute only rear their ugly head if you try to copy all of the non-specified values; Like “height” which in IE5+6 is total gibberish because it’s a system var that’s more offsetHeight+allParentsOffsetHeight on read, but ends up style.height in px on write (doh)… or “dataFormatAs” which never gets set and is read-only. IF you check ‘specified’ - end of problems using it in IE.

The “big problem” is that as it sits right now it doesn’t copy the ‘event’ values, those would have to be hard-coded… I hate hard coded and there’s no guarantee those will work either when the original is destroyed. Were that we could just cloneNode, but a clone won’t let you set ‘type’. (I’m wondering if one of the hundred+ attributes returned in legacy IE actually can be used to fix that)

See what I said in my commented version:

“the indexOf is a bit sloppy and could have a false positive
but the likelyhood is low and it’s faster than a regex.”

I agree it’s problematic - Really what should probably be used there is getElementsByClassName with the legacy workaround script.

I’m finding that people are WAY too paranoid about that these days… though that’s why I use a underscore prefix in the middle of my camelbacks; simple search and replace if I’m merging it with a script that could have a conflict. I keep the variables separate because most likely I’d be copying this into a larger library.js containing all the script specific to the site, and would want things that are edited for the site at the top, and functions at the bottom. Besides, if you put all your scripted globals at the top, you can SEE if they are going to conflict. Hardcoding it into the function becomes a headache once you have multiple functions present. (of course it’s a shame we don’t have strict typecasting, locked constants and forward declaration to help diagnose such issues in the first place)

The ‘namespace’ obsession has done little more than result in people throwing the overhead of objects into their code for no good reason, and crumple up maintainability into a little ball and toss it in the corner with that wad of lint-covered gum and the Altoids mint tin that still has mints in it from 1984. It’s not a bad idea to think about, but calling two variables “polluting the global namespace” is taking it to absurd extremes.

But then, some people say the same thing about how I write my PHP. :stuck_out_tongue:

Interesting approach, though it has a few issues (or at least issues for me)… Where you insertBefore the label (line 10), you don’t remove the original label which is there for when javascript is off… previousSibling.replace?

Oh I see, you’re using the second label to show the scripting only text explanation instead of giving it the short version… That works too.

I’m NOT wild about sending the element replace to all browsers, specifically safari and Opera, both of whom have the habit when you replace an element in a form, they have the nasty habit of erasing the contents of some other input (I’ve never tracked that down) which is part of why I prefer to just set ‘type’.

Also, yours only works off a fixed element – easy enough to fix with a function wrapper… at which point I’d move the called function into a standalone so the parser doesn’t have to go through it every time you call it (should you want more than one input[password] field handled that way on a page… it also has a BIG flaw, no hooks to apply any of the styling to it with.

Still, some good ideas in there. 1.1k, probably like 1.4k once the missing functionality is added. Gonna play with that for a bit.

Taking the above suggestions and those in the sitepoint article’s comments into account, I’ve redone it over again. This time no InnerHTML or document.write as the arguements against it made some sense, especially once I decided I wanted it to go through the document and apply it automatically using a class trigger.

Same URL’s as before. (be sure to flush to get new script/new CSS)

Demo Page:
http://www.cutcodedown.com/codeExamples/showPasswordCheckbox/demo.html

Unlocked Directory:
http://www.cutcodedown.com/codeExamples/showPasswordCheckbox/

The script itself:
http://www.cutcodedown.com/codeExamples/showPasswordCheckbox/showPasswordCheckbox.js

and I added a commented version of the script explaining the choices I made and how it works.
http://www.cutcodedown.com/codeExamples/showPasswordCheckbox/commented_ShowPasswordCheckbox.js

Ease of application for those not script saavy while maintaining graceful degradation was the name of the game this time. Implements the @cc_on drop-through for the onclick and the try/catch on the .type assignment Raffles suggested (as I said in the comments for it – SLICK), but has all the ruffles and flourishes of not needing to be hand-fed which ones to apply to. Just put the sp_password class on the INPUT you want the extra stuff applied to, and BAM.

I also parse the attribute tree for IE, checking for .specific and making sure VALUE is copied since IE7/earlier doesn’t always update that value when the user types in a result.

It’s a wee bit bigger at 2070 bytes, but loses that pesky extra handshake I had (you guys were right, not worth it), shakes off the ID on the labels, and you don’t have to call the init function for each and every password field on a page.

Ah, that felt good. Needed something to help shake off the ring rust. <flair>WHOOOOO.</flair>

Yeah I missed that until I had it in Crimson and started ripping it apart. What you did there is actually quite slick, it just wasn’t apparent to me at first.

I was actually discounting the ID’s since I’d be writing it to possibly have more than one instance on a page. Mostly though I was referring to the lack of anything to format it for CSS off and/or a block level parent container… Though the original had that problem too.

You gave me some good ideas, and I ran with it… Opinions on my new version?

OK, so you find it easier to use with. It isn’t difficult to understand, but it’s largely avoided and looks odd, and so therefore is unfamiliar to most people. There is something to be said for coding conventions and I think this is one that exists for a reasonable enough reason.

Nice demo page and much better kept all in one script. The big issue I have is with your use of the with statement. It’s one of those that’s entered the “considered harmful” club.

In fact, your code is a perfect example of why with is bad. It makes the code harder to read and, more importantly, you have nested them, which makes the ambiguity Crockford talks about obvious:

with (document) {
  // ...
  with (extraText=createElement('em')) {
    className='extraText';
    appendChild(createTextNode(sp_extraText));
  }
  // ...
}

In that second with block, you’re doing stuff with extraText.className, extraText.appendChild but then document.createTextNode! I’m surprised that code even works - I’d have expected the browser to interpret it as extraText.createTextNode, which would cause an error. At one point, you’re nesting with statements four deep!

You should also avoid using the attributes collection. Note the obvious grey “Do not use” box.

A minor niggle is this:

if (inputList[t].className.indexOf('sp_password')!=-1) {

Since you’re doing this with every single input on the page, if someone happens to have an input with something like “jsp_password” as the class, it’ll go through. Better to use split(’ ') or a string match using a regular expression.

You should also avoid polluting the global scope with those two first sp_ variables and simply put them inside the function, they don’t need to be outside it.

I’m NOT wild about sending the element replace to all browsers, specifically safari and Opera, both of whom have the habit when you replace an element in a form, they have the nasty habit of erasing the contents of some other input (I’ve never tracked that down) which is part of why I prefer to just set ‘type’.
No, that’s not what will happen. That’s the point of the try…catch. IE cannot dynamically change the type attribute on INPUT elements, which is why the try block will fail, but only for IE. However, Opera, Safari and Firefox can deal with that, so they don’t even go into the catch block. Therefore as far as non-IE browsers are concerned, the code is very small and could be as simple as this:

var passbox = document.getElementById('demoPassword'), fieldset = passbox.parentNode;
fieldset.innerHTML += '<input type="checkbox" id="showPassword"><label for="showPassword" title="Show the password as plain text (not advisable in a public place)">Show password</label>';
document.getElementById('showPassword').onchange = function() {
    passbox.type = this.checked ? 'text' : 'password';
}

it also has a BIG flaw, no hooks to apply any of the styling to it with
Er, I wouldn’t go that far. This is about the JS, not the styling. It’s a trivial matter. I don’t see what you mean, there are more than enough hooks - practically every element has an ID! If anything, I’d remove some of them.

Also, yours only works off a fixed element – easy enough to fix with a function wrapper
Of course… but this is also pretty trivial.

See, I find saying it over and over again more confusing… More code, harder to follow… But I learned “with” on Modula/Pascal where you have complex data types even BEFORE you get into objects - to me it’s second nature. (also why objects in Modula/Pascal feel ‘natural’ and in C Syntax feel like trying to cram a 500 pound fat woman’s cankled stump into a size 7 shoe – RECORD beats the crap out of STRUCT)

I actually often have a hard time “dumbing down” my coding techniques because I often do things based on an understanding of how the interpreter works under the hood (or should work if the person who wrote it has any clue what they are doing… a bad assumption sometimes) – you’ll notice that the scope of the innermost nested ‘with’ is barely four lines and only one variable call is to the parent with, the rest being to the local one.

Also, I won’t use a WITH nesting if it ends up more than 24 lines tall… don’t know WHERE I came up with that number :wink: If it doesn’t fit on a 80x25 screen all at once… and if your editor can’t show you more than 25 lines at once (and use two space tabs) you’re probably in the wrong editor :stuck_out_tongue:

Using with is fine with compiled languages but is extremely slow with interpreted languages such as JavaScript because it has to carry out a search to work out which particular field is being referenced EVERY time a field within the with statement is referenced.

The reason for avoiding using with in JavaScript is not to do with convoluted code, it is to do with the code running way way slower than it would without it. With four nested with statements the code is perhaps running at 1% of the speed that it could without those statements and referencing the fields directly.

calling two variables “polluting the global namespace” is taking it to absurd extremes

Fair enough. But my comments on with still stand, because you’ve designed this script to be something someone can just drop into their own code. with is sloppy and makes code harder to read. Even with a short script like yours, I found myself having to backtrack to remember what object was being worked on.

A minor variant on my self labelling password script at http://javascript.about.com/library/blpass1.htm would be how I’d go about it. Almost all the code there is specific to IE - the part not shown in bold.

or have a ton of DOM manipulating scripting to do “insertAfter” (nextSibling exists then insertBefore, nextSibling does not exist then parentNode.addChild – WHY is this not a standard function?!?)
It is - insertBefore can also work with nextSiblings that don’t exist, to cater for “insert right at the end” cases.

I had a go at this, and came up with the following. I strongly disagree with the use of document.write and with having three different .js files just to achieve something really quite simple. If your argument is that you don’t want to “pollute” the javascript sent to good browsers with the IE-only stuff, it’s moot, because you’re forcing them to make an extra HTTP request for the non-IE .js file.

The image I think should be a background image on the label element. Using your original HTML, this works:

var checkbox = document.createElement('input');
checkbox.passbox = document.getElementById('demoPassword');
var fieldset = checkbox.passbox.parentNode,
    label = fieldset.insertBefore(document.createElement('label'), fieldset.lastChild.nextSibling);
label.appendChild(document.createTextNode('Show password'));
label.htmlFor = 'showPassword';
label.title = 'Show the password as plain text (not advisable in a public place)';
checkbox.type = 'checkbox';
checkbox.id = 'showPassword';
fieldset.insertBefore(checkbox, label);
/*@cc_on checkbox.onclick = @*/ checkbox.onchange = function() {
  try {
    this.passbox.type = this.checked ? 'text' : 'password';
  }
  catch (e) {
    if (!window.event || window.event && window.event.type === 'change') return;
    var newbox = document.createElement('input'),
        attrs = ['size', 'maxlength', 'value', 'name', 'id', 'className', 'title'];
    newbox.type = this.passbox.type === 'password' ? 'text' : 'password';
    for (var i = 0; i < 7; i++) {
      newbox[attrs[i]] = this.passbox[attrs[i]];
    }
    fieldset.replaceChild(newbox, this.passbox);
    this.passbox = newbox;
  }
}

With innerHTML of course it gets even smaller:

var fieldset = document.getElementById('demoPassword').parentNode;
fieldset.innerHTML += '<input type="checkbox" id="showPassword"><label for="showPassword" title="Show the password as plain text (not advisable in a public place)">Show password</label>';
document.getElementById('showPassword'). /*@cc_on onclick = @*/ onchange = function() {
  try {
    document.getElementById('demoPassword').type = this.checked ? 'text' : 'password';
  }
  catch (e) {
    if (!window.event || window.event && window.event.type === 'change') return;
    var newbox = document.createElement('input'),
        passbox = document.getElementById('demoPassword'),
        attrs = ['size', 'maxlength', 'value', 'name', 'id', 'className', 'title'];
    newbox.type = passbox.type === 'password' ? 'text' : 'password';
    for (var i = 0; i < 7; i++) {
      newbox[attrs[i]] = passbox[attrs[i]];
    }
    fieldset.replaceChild(newbox, passbox);
  }
}

And I don’t think jQuery is hard to read. As long as you know what each method does, it’s very readable. I’d argue it can be easier to read than vanilla JS, especially if there are loads of document.getElementByWhatevers and document.createWhatevers sprinkled all over the shop. And come on, of course it’s easier - that’s the point of jQuery! It handles cross-browser nightmare cases like these without you having to even think about it.