miscoded

I thought the analysis of the US Airways login issue would be best presented as an illustration of the code paths. Not that I'm an expert at UML, but Lucidchart made it pretty easy to draw this (as a bonus, I saw a few issues that might be Opera bugs and I'll go analyse them shortly).

So, we're looking at a script that wants to handle the enter key and call form.submit() itself, hence it needs to block the default "pressing enter submits form" action of the web browser. Note also that if you submit the form by pressing enter, the form's submit event will fire. The site is apparently confused because the submit event fires in Opera when you press enter, while it doesn't in other browsers. Why does the script fail to prevent the default action of the enter key?


This chart shows the code execution paths of the different browsers - how IE and Opera go on a detour by running the javascript: URL immediately, while Firefox and Chrome just finish the current thread.
For reference, here is the code again - now slightly re-ordered with the functions appearing in the order they are called:

<form method="post" action="#" onsubmit="javascript:return original_onsubmit();">
	<input type="text" value="Press Enter">
</form>
<script type="text/javascript">
if(document.addEventListener)
	{
	f.addEventListener('submit',foobar,false);
	userName.addEventListener('keypress',loginkeypress,false);
	}
else if(document.attachEvent)
	{
	f.attachEvent('submit',foobar);
	userName.attachEvent("onkeypress",loginkeypress);
	}

function loginkeypress(e)
	{
	e = e?e:window.event;
	if(e != null && e.keyCode == 13)
		{
		if(e.preventDefault)
			{
			e.preventDefault();
			}
		else if(window.event)
			{
			e.returnValue = false;
			}
		document.location = 'javascript:submitform()';
		return false;
		}
	}
function submitform()
	{
	if(f.onsubmit() != false)
		{
		f.submit();
		}
	}

function original_onsubmit()
	{
	if((typeof(window.event) != "undefined") && (window.event != null))
		{
		window.event.returnValue = true;
		}
	return true;
	}


function foobar()
	{
	alert('FAIL');
	}

</script>

And a textual description - for those of you who disable images :

• kicking off from the keypress thread, we see that the keyCode is 13 and call preventDefault() to avoid submitting the form
• within the event handler, we set document.location='javascript:submitform()'. This executes immediately (in IE and Opera - timing of JS URL execution test case), pausing the event thread to go into submitform()
• inside submitform() we find the curiosity
  

  if(f.onsubmit() != false)

  
  Now, this actually calls one of the form's two submit handlers directly (there is one from the onsubmit="" attribute and another one from the addEventListener call). So, into it we step, and..
• here it starts messing about with window.event
  

  if((typeof(window.event) != "undefined") && (window.event != null)) 
  { 
  window.event.returnValue = true; 

  
  presumably believing that window.event is the event object from the submit event. But remember, this stuff started with a *keypress* event and the call to onsubmit is not from Opera's internal event processing - it's from a JS URL running from "inside" the keypress.. So window.event is the event object of the keypress event, and setting returnValue=true effectively cancels the earlier preventDefault() call.

So when all is said and done, the javascript: URL has called form.submit() and returns to finish off the keypress event thread, the event is not cancelled after all, it triggers another form submit and thus the submit listener from the addEventListener call.

If IE and Opera follow the same path (except for IE not supporting preventDefault()), why doesn't the same thing happen in IE? Because of another quirk: in IE's event handler model, the return value from functions added with attachEvent() matters. It is significant that the loginkeypress function returns false (see returning false from event handler test case). Since the event handler in Opera was added with addEventListener(), what the function returns is ignored. Hence, in IE the state of the enter key's default action is toggled three times: disabled, enabled and then disabled again, while in Opera it's disabled and then enabled again.

To debug this, I used mostly opera.postError() / console.log(), some Dragonfly (in this case my brilliant colleague Kåre already did the harder part - extracting the problematic code from the live web site.). As the main problem was whether the key's default event should be disabled, adding quick postError() calls where the script set returnValue or called preventDefault() was the fastest way to understand the code flow. (Stepping through a script is more useful if there are many variables you would like to examine while the script runs. If you just need to understand the code flow, logging is usually faster than stepping).

As I always start debugging in Opera, I tend to litter scripts with opera.postError() calls. With a problem like this, where much of the work actually is figuring out why it works in other browsers, this can be a bit inconvenient. Luckily, I don't need to spend time on replacing all postError calls with console.log before running a test case in Firefox or Chrome - I simply paste this snippet above the code:

if(!window.opera){
	if(!window.console){var opera = { 'postError' :  function(){ for(i=0; this.OperaPostErrorsOK&&i<arguments.length;i++){this.OperaPostErrorsOK = confirm(arguments[i]); }   } , 'OperaPostErrorsOK' : true  };
	}else{var opera={'postError':console.log}}
}

and opera.postError() works cross-browser

Once we've understood the problem, the next step is naturally to find a way to solve it and make Opera work with the site. If the problem comes down to us breaking a spec or being generally buggy, how to fix it is obvious. With a problem like this, however, where all parts of the puzzle are basically by design and the issue is caused by an unfortunate combination of correctly supported features, finding the right fix is harder...

Ways we might solve this (doing just one of these would fix the site):

1. make returning false from event listener cancel the default (even if the event listener was added with addEventListener() or attachEvent() )
2. drop support for event.returnValue
3. make preventDefault() "override" event.returnValue, if the former was called setting the latter to true is ignored
4. un-set window.event while we execute javascript: URLs
5. change scheduler to run javascript: URLs after current thread rather than immediately

Doing 1 means mixing even more of the IE-model's event handling logic into the standardised API - and we have absolutely no idea how much this would break, it would be a very risky change in terms of site compat.

2 is in line with other on-going attempts at dropping IE-specific event stuff (attach/detachEvent) but perhaps a bit premature. Also, IMHO event.returnValue is a nicer API than event.preventDefault() because the latter can not be undone. Doing this might have a small site compat impact: we might break scripts that rely on event.returnValue, but we believe that few scripts do since Firefox doesn't support it.

3 and 4 would be Opera-specific hacks. I find it highly unlikely that we would break anything, but we would diverge more from the other browsers in extremely subtle ways.

5 has a high risk of regressions - changing the order we run scripts in, is a deep change and in the past we've always needed several months to find and fix regressions. These regressions can also be extremely severe (for example the problem that prevents adding Facebook notes - now sitepatched in browser.js - is related to scheduling and probably a regression from changes made more than a year ago). This fix will however align our scheduling with Firefox and WebKit. (WebKit has very strange quirks here though - it seems for example calling location.replace('javascript:...') will make it ignore scripts in the rest of the document, or something like that.)

I recommend fix 5 - probably the most complicated fix, but in the long term the right thing to do.

...all that hard work because of a script that does entirely pointless and nonsensical things..?

That's browser QA in a nutshell. We're used to it.

The Rabobank site in the Netherlands has a peculiar problem caused by an even weirder script. The forum discussion explains what the problem is: the site's search box doesn't allow you to type the letter T!

This is caused by a keypress handling JavaScript - it's all on one line so I made a re-formatted copy. If you thought the bug was weird, wait till you try figuring out what the point of the script is..

The basic logic that causes the problem is: "any browser that supports addEventListener() will support charCode but not keyCode in the keypress event". As it happens, Opera does support addEventListener() but not charCode. The site blocks keypress events with keyCode 116 which is the correct keyCode for a "t" keypress. (If we didn't support addEventListener they would listen for keyDown events with keyCode 116 instead - which is what they basically intended to do.)

With the problem analysis out of the way: I simply don't get what this site is trying to do. They set one handler for keypress events to cancel them if it's the F5 key, and another handler for keyup events to call location.reload() if it's the F5 key!? See here:

function F5DownEventHandler(evt){
	this.target=evt.target||evt.srcElement;
	this.keyCode=evt.keyCode||evt.which;
	this.altKey=evt.altKey;
	this.ctrlKey=evt.ctrlKey;
	var targtype=this.target.type;
	if(this.keyCode==116&&(evt.charCode==null||evt.charCode==0)){
		return cancelKey(evt,this.keyCode,this.target)
	}
	if(this.keyCode==13&&(this.target.id=='z01'||this.target.id=='z02'||this.target.name=='z5'||this.target.id=='ra_searchfield2'||this.target.name=='z81')){
		return cancelKey(evt,this.keyCode,this.target)
	}
}
function F5UpEventHandler(evt){
	this.target=evt.target||evt.srcElement;
	this.keyCode=evt.keyCode||evt.which;
	this.altKey=evt.altKey;
	this.ctrlKey=evt.ctrlKey;
	var targtype=this.target.type;
	if(this.keyCode==116&&(evt.charCode==null||evt.charCode==0)){
		window.location.reload(this.ctrlKey)
	}
	if(this.keyCode==13&&(this.target.id=='z01'||this.target.id=='z02'||this.target.name=='z5'||this.target.id=='ra_searchfield2'||this.target.name=='z81')){
		this.target.parentNode.parentNode.submit()
	}
}

So: "we don't mind that [F5] reloads the page and [enter] submits forms as long as we re-implement the browser's functionality in JavaScript".

Or: Rabobank trusts only Rabo-programmed keys?

I guess the browser's OWN implementation of [F5] and [enter] just wasn't buggy enough.

If you use Google Maps with Opera 9 betas you have probably noticed that the top left navigation does virtually nothing. Only the slider works at the moment.

Technically, they use an empty element that is styled to cover the graphic to catch mouse events and call the navigation actions. (Don't ask me why it is coded like that, and why for example the graphic isn't the background of the DIV so that this problem doesn't arise).

This not working in Opera is caused by our experimental support for event transparency.

At first sight I thought I had missed something in IE's implementation that would make Google Maps work. However, none of my tests could identify the missing piece of that puzzle - and it turns out what they actually do is to set a background colour on the empty element and then make it invisible again by setting an opacity filter!

Now, there is no natural link between event transparency and opacity. If a UA implements the former but not the latter, our Google Mappers would have to look for another workaround. It is pure luck that Opera happens to add support for both at the same time, so Google will fix this by using the IE workaround for Opera too.

This issue shows how important it is to standardise a way to say an event should fire on invisible elements. I think addEventListener should be extended with a fourth argument that controls this.

As an aside, to avoid browser sniffing you can use this code to detect support for CSS3 opacity in UAs that support the DOM standard's getComputedStyle:

var el=document.createElement('div');
el.style.opacity='.5';
var opacitySupported=(getComputedStyle( el, 'opacity').opacity)?true:false;