miscoded

The Yandex.ru webmail monitors itself and sends information about certain unexpected error conditions back to the server.

Most servers would just shrug and record the error. However, mail.yandex.ru seems a bit more..secretive about it. Perhaps because secrecy is a Russian thing, perhaps to amuse browser QA, or perhaps they just try to take web debugging to another level..? Here is the full text/plain document they return when the script says "Oops, an error happened":



Ssshhhh! Don't tell anyone.

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.

We recently investigated a bug report about this US Airways login form. Pressing enter to log in fails - but only in Opera. The question is why? Read on if you want to try to analyse the problem..

Read more...

This is an excerpt of what Firefox gets when loading Amazon music sampler:

var amznJQ = {
        _a: [] , _s :  [], _d: [], _l: [], 
_o: [], _c: [], _cs: [],
        addLogical: function() { 
            this._l.push(arguments) 
        },
        addStyle: function() { 
            this._s.push( arguments) 
        },
        declareAvailable: function() {
            this._d.push(arguments)
        },
        available: function() {
            this._a.push(arguments)
        },
        onReady: function() {
            this._o.push(arguments)
        },
        onCompletion: function() {
            this._c.push(arguments)
        },
        completedStage: function() {
            this._cs.push(arguments)
        }

If Opera asks for that page, we get this:

var amznJQ = {
        addLogical: function ( ) { },
        addStyle: function() { },
        declareAvailable: function () {},
        available: function() {},
        onReady: function() { },
        onCompletion: function () {},
        completedStage: function() {}[/url]
That can only be described as [i]surgical[/i] browser sniffing: very carefully make sure functions are empty and variables not declared if the browser is not on your list of friends. Hey Amazon, browser sniffing is uncool even though you pretend to be surgeons :(