search

itswebny / universal-navigation

Guidance and implementation instructions relating to the Universal Navigation (formerly NYS Common Web Banner) deployment.
24 stars 10 forks source link

Improve frame buster code #171

Open patik opened 9 years ago

patik commented 9 years ago

The frame buster page could use a couple of improvements. The following items are not only good practices, but they are used in the interactive embed code.

  1. Wrap the JavaScript in a function so the variables don't leak to the function scope
  2. Add the script to the document using DOM methods rather than document.write() which can be harmful
  3. Check that the regular expression actually found a match for the parenthetical group. The .exec() method will return an array of 1 item if the overall pattern matches, even if a group inside the pattern (in parentheses) doesn't match. The code below that relies on the array having at least 2 items.
  4. Add a doctype. Even though browsers can get by without one, they each have their own idea of how to handle quirks mode. Using a doctype puts them (mostly) on an even playing field.
  5. Add a <head> element, even if it's empty. I know that document.getElementsByTagName('head') will work in modern browsers regardless, but I'm not sure about legacy browsers and it doesn't seem worth the minimal bandwidth to leave the element out.

I'm opening a PR to address these but I wanted to outline the changes and rationale here.

esteinborn commented 9 years ago

The minimum viable code an html5 document needs is:

<!doctype html>
<html>
<title>a</title>
<body>
</body>
</html>

everything else is butter.

and :+1: to handling this now instead of after everyone deploys.

How are we testing this to ensure it still works in all browsers?

patik commented 9 years ago

Iceboxing this for now, per @lukecharde.

esteinborn commented 9 years ago

To be clear, to anyone happening upon this issue, even though we are iceboxing, we still believe this issue is important. It will be more important than ever to begin unifying our coding standards for things like this. Especially considering that we will need to be the owners of this content in the end.