US Airways enter issue explained

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.

Advertisements

34 thoughts on “US Airways enter issue explained

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

    That's life…I also agree that option number 5 seems like the best one here since that will make Opera behave the same way other "good" browsers do (IE is always its own quirk, but goodness is subjective, hence quoted), but I suppose short-term fix in browser.js would be at least easier. That change does indeed seem like it could have a very big impact in the design of how Opera handles these things and will very likely bring some regressions = more work to find out how to fix them.Nice explanation anyway. It's always interesting to read your blog and get insight into what's going on with the evil web. πŸ˜‰

  2. Anonim writes:id start from 3, check using your magic tools (you had a crawler of sorts) what other popular/known to be misbehave in opera sites are using returnValue and then proceede with 5if my intuition does not fool me, this returnvalue thing is a big mess and reason of some sites 'dying' when they definately shouldnt.give it a trybtw. have you found what causes google maps to die (not able to pan with click-drag) after like 5 minutes of dragging? (other controls work, but click-drag dies (and the cursor is no longer 'a hand') it is 100% reproducible and a quite old bug.

  3. Seems very complicated and hard to fix, but this

    as a bonus, I saw a few issues that might be Opera bugs

    and fixing another little piece of compatibility to the web should be worth the effort.But in the end whoever else should fix it, if not the best? πŸ˜€

  4. Thanks for re-ordering the original script. Laid out in order, it makes much more sense. Like I said, I don't know enough about JS, so explanations like this are very helpful.I might almost propose a 6th option: Since Opera likes to submit on Enter, lobotomise the login script entirely using browser.js….not very nice when people write many lines that do almost nothing.

  5. This story also shows why "compatibility" is so hard. Each separate feature this script uses was implemented with compatibility in mind, – each fix we attempt here probably will break another website somewhere (and we'll just pray that the other website is not an important one).Compatibility is the binary product of uncountable details.

  6. Originally posted by anonymous:

    have you found what causes google maps to die (not able to pan with click-drag) after like 5 minutes of dragging?

    I'm not aware of this issue. Couldn't find any open bug report on a quick check (there might be one I wouldn't find by searching for "google maps dragging" of course..)Perhaps you could report it?

  7. Anonymous writes:Hallvors. || I would report it, but some Opera employee 'moderated' my thread on my.opera on this very topic by deleting it some time ago. Given that attitude I leave it to Opera itself to fix their own bugs, fill them and gather necessary info. I know, that it wasnt you (probably) but damage is done and I'm not willing to do unpaid job for company that is afraid of admitting to its own shortcomings. Case in point: on desktop team blog, there was a user banned just for explaining why opera takes ages to close (gigantic cache not respecting 'max values' while running and then taking ages to delete the files). With this attitude, good luck. Btw. this gmaps issue is a deal breaker if you use it regularly and is quite easy to spot.

  8. Originally posted by anonymous:

    but some Opera employee 'moderated' my thread on my.opera on this very topic by deleting it some time ago

    Hm, that's weird 😦 Haven't found the deleted thread (I seem to have access to deleted threads from some but not all fora) so I have no idea why. It's certainly to our loss if My Opera moderation is so draconian that real issues go under-reported.I'll admit that though I'm a sporadic Google Maps user, I've never been dragging a map for five minutes ;).

  9. Trying to drag myself [1] from Fukuyama along Sanyo highway and Shimanami kaido highway towards Imabari I could eventually reproduce! Finally found the problem o/ – now, I can tell that analysing this will take some time, especially since it took me at least 10 minutes to get Opera into this state in the first place :-/ I'll report the bug now, and we'll try to figure it out.[1] I'm glad it's just a virtual map. If I were actually dragging myself from Fukuyama to Imabari there wouldn't be much left of me :-p

  10. Originally posted by anonymous:

    'moderators' acted again

    I just saw that a mod also edited this comment. It was not a racist comment, as stated, but was maybe a bit too critical or anti-opera for the moderator. I know because I answered this comment (my comment was deleted). I dont think its the right way to just delete critical comments or disliked opinions or even pretend them to be racist ones.(Of course I could be wrong and he edited his comment to a racist one, forget what I said if this would be the case)Sorry, I dont want to be too offtopic in here.

  11. Anonymous writes:Thanks for your effort. Desktop team 'moderators' acted again: http://my.opera.com/desktopteam/blog/show.dml/17804132?startidx=150#comment41482642 – if you have access to pre-moded content you'd be shocked what was inside. A diagnosis why multi-file cache + AV software slows down opera, when opera accesses it all on startup. I really wonder who is designed to do mod tasks, but they are doing WAY more bad than good. || As for the maps issue, I played with it today, it takes like 3 minutes of constant panning to break it in opera (and opera only) (during my post vacation photo tagging session I used FF 3.6 for straight 4 hours on the same maps page, and it did not break). Imagine dragging from Goa in west India to Bejing in China πŸ™‚

  12. Anonymous writes:I saw that comment to, it wasnt racist in any f.. way. Others were banned for 'breach of service use', 'degragatory comments' and other jokes – all lies. I dont know who pay these moderators, but each and every user treated that unfairly is an ex-user.

  13. For the Desktop Team blog, mods have been cracking down on anything even remotely off-topic no matter how good the post is. For example, my comment here would be deleted as it's off topic.In short, on the DT blog, you're not allowed to deviate from the discussion at all. There's no freedom to do that. It's stay 100% on topic or GTFO. This is unfortunate, but it helps devs/qa process comments easier.For forum posts, besides the constant spam (the forums have been getting hit *hard* lately), you can tell that some mods (or some people) do indeed delete posts only because they have personal issues with them. Then again, some posts are just totally off-topic for the forum they're in or they're flame bait or totally subjective. Of course, whoever deletes/closes the post could give better reasons.But, most of this stuff could be solved with a tag-based forum and options to bury comments and threads with a voting system etc. The forums are just too brittle and old-school for that right now. Hopefully things will be improved in the future.In short though, I doubt it's a bunch of mods being mean and it's probably only one or two.

  14. Anonymous writes:Oh and btw – you are of course allowed to deviate off topic on Desktopteam – if you are prd3, or any other pro-opera troll. He is left to spread hate, insults and derogatory comments at will. That makes you think.

  15. Anonymous writes:"In short, on the DT blog, you're not allowed to deviate from the discussion at all. There's no freedom to do that. It's stay 100% on topic or GTFO. This is unfortunate, but it helps devs/qa process comments easier." ||||||Problem is that mods – in this recent topic about opera closing slowly – went berzerk and deleted/banned people that were contributing the most to the discussion. People that know what makes opera slow (it isnt rocket science, any half assed Win developer knows how to use Sysinternals FileMon – it is BASIC stuff) and describe that in detail. That opera does not (nor ever did) respect 'max cache size' and thus produce gigantic cache structures that take ages to delete (log cost due to multiple traversals). And that Opera leaks pages with time (Opera closing time is linear with time being open – try to close it after a day of usage..). People are amazed why stuff that is KNOWN in the forums for literally YEARS is now 'researched'. And for asking this question they all got deleted. Just like mine topic about google maps dying in opera after some use.

  16. Originally posted by anonymous:

    I saw that comment to, it wasnt racist in any f.. way.

    I wonder what your agenda is, since you keep lying. The comment was clearly racist, talking about certain people in very derogatory terms, as if they had less value as human beings just because of their background. True racism, in other words.

  17. Originally posted by burnout426:

    you can tell that some mods (or some people) do indeed delete posts only because they have personal issues with them

    Nonsense. The mods here are far more lenient than, for example, the mods over at the Mozilla forums. You seem to want anarchy.

  18. Originally posted by anonymous:

    roblem is that mods – in this recent topic about opera closing slowly – went berzerk and deleted/banned people that were contributing the most to the discussion.

    Really! And who was banned, exactly?List these people who "contributed the most and were banned", or apologize for spreading blatant lies.

  19. Originally posted by burnout426:

    In short, on the DT blog, you're not allowed to deviate from the discussion at all. There's no freedom to do that. It's stay 100% on topic or GTFO. This is unfortunate

    BS. It's necessary because of the sheer amount of trash being posted. How many times are they going to have to tell people that flooding the blog with trash is only going to bury legit feedback?

  20. Originally posted by prd3:

    It's necessary because of the sheer amount of trash being posted.

    Indeed. For example, with the latest post for example, people were explicitly told not to post their logs in comments.Originally posted by prd3:

    Nonsense. The mods here are far more lenient than, for example, the mods over at the Mozilla forums. You seem to want anarchy.

    We already have that on the DT blog. But, I think there could be better ways to handle things.Originally posted by prd3:

    How many times are they going to have to tell people that flooding the blog with trash is only going to bury legit feedback?

    ∞. We all have A.D.D. Expecting everyone to stay on topic is like expecting 2-dimensional beings not to fly into cosmic strings. It's instinctively impossible. Typical moderation doesn't really solve anything and can be insulting. There might be a better and nicer way that's worth exploring. Either that or it's time for mods to get mean, strict and nasty.

  21. *sigh* like any good troll, just set a pie by the window and a snare 'neath the sill….mention prd3 and in it flaps on leathery wings…Hallvord: Thank you very much for taking a fresh look at the map-dragging issue. I just noticed it today, trying to plan a big trip in mid-winter.Please do not set a voting system. I've seem enough articles on the systematic manipulation of stories on Digg.

  22. Jeri writes:Congratulations to the trolls for successfully derailing this informative blog post. I'm always amazed to see how people will push their personal agenda, derail an informative article, and get everyone else to jump on their bandwagon of lies and deception too.

  23. Anonymous writes:Jeri || Regardless of what you think about trolling or not, it is not the post that get 'derailed' but the comments. Post is still there, and noone made it any worse. || Hallvors – can you disclose what approach will Opera take in this case (and why, if that isn't some sort of a secret)?

  24. Jeri writes:10-15 of the comments here are about other things than the blog post. I see nothing but a bunch of noise, and someone derailing an interesting subject with unsourced claims about other things. I consider that to be trolling.

  25. Originally posted by prd3:

    Originally posted by anonymous:

    roblem is that mods – in this recent topic about opera closing slowly – went berzerk and deleted/banned people that were contributing the most to the discussion.

    Really! And who was banned, exactly?

    List these people who "contributed the most and were banned", or apologize for spreading blatant lies.

    So, are you going to answer the question?Of course, if you can't show specific examples, we must assume that you are lying again.

  26. I understand that there are some strong feelings regarding My O moderation – I would like to close this discussion and get back to the topic of this blog post. On this blog I usually only censor spam but sometimes I've also removed what I found was overly insulting statements towards another participant in the discussion. I won't start censoring this discussion but I would like to ask you to kindly take it elsewhere.Originally posted by anonymous:

    Hallvors – can you disclose what approach will Opera take in this case (and why, if that isn't some sort of a secret)?

    Nobody started working on a fix yet, but I've recommended solution 5 from the post above, and I'm pretty sure we'll ship that. Over time we're edging closer to Firefox/WebKit in terms of feature support, so some day we might also remove event.returnValue as a result of that process (so regarding this bug we'll likely kill one bird with two stones ;)). I'm always concerned that we'll break sites when we remove stuff, but some creative automated compatibility testing seemed to work pretty well last time we dropped an IE-compat hack :).

  27. (BTW, the lucidchart.com issue is analysed, and so is the Google Maps dragging issue. Both were already known problems and there is ongoing work to fix them – but the Maps issue in particular will take a while!)

  28. Anonymous writes:Thanks for the update, esp. for the google maps. I guess that lack of dev tools and google' obfuscated and crazy code is the reason for it to be so tedious. (it is crazy even when it is plain, and you have dynatrace tools..). || with the returnValue I'm happy to hear that you as a company recognize need to be similar to others in gray areas. but I would reconsider doing point 3 before attempting 5. Reason? the longer you wait with 5, the more pages relying on this 'bug' in opera will be there.. and once they are in, they are almost never out

  29. Sometimes I wish the context menu had a "page tools" expander which contained "View source", "Validate" "Report a site problem", "Open with:", and 1-or-2 others. Reduces clutter, makes site-reporting easier to access. Is MAMA scheduled for another run, or does Opera have some similar mechanism now actively in place…?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s