miscoded

Launching in browser.js tomorrow or so: OddPatch beta, solving many of the compatibility issues with the service formerly known as OddPost, now Yahoo!Mail beta.

It's a complex patch sorting out a daunting number of issues on both sides. When things get this complex, it's rarely only "their fault" or "our fault" - the testing has uncovered several bugs in Opera and several mistakes in their JavaScript. Here's a walk-through of the entire patch:

// browser sniffing workaround - walking in through the back door
if( location.href.indexOf( '/dc/system_requirements?browser=blocked' ) >-1){
location.href='/dc/launch?sysreq=ignore';
}

Y!Mail has three modes for browsers: supported, possibly working, or blocked. Opera is blocked, but luckily there is a backdoor that will bypass the sniffing.

The irony here is that when they block us, they are making their work on Opera-compatibility much harder than necessary. If we get access, we'll do our best to make things work: test, find bugs, even decide to support things we haven't supported previously (we'll have selectSingleNode soon because Y!Mail uses it heavily, and their code was also a very important reason why DOM2 Style support was prioritised for Opera 9.0!). Blocking us makes it much harder for us to make their life simpler.

if( top.location.href.indexOf('/dc/launch')>-1 ){ 
// Gecko compatibility library uses defineGetter and defineSetter. We need to fake them.
//* Patch below is required but causes trouble.. 
Object.prototype.__defineGetter__=      function(){}
Object.prototype.__defineSetter__=      function(){}

This is known as "fake it until you make it". We won't have getters and setters anytime soon, but things will work anyway if we pretend we do.

// IEism called loadXML, basically a DOMParser / DOMLS equivalent
// must handle XML fragments without root element!
Element.prototype.loadXML=function(s){  
try{
var d=new DOMParser().parseFromString(s, 'text/xml'); 

This, I think, is a bit of the strange world IE lets you into if you put an XML tag in a page. That tag takes on a life of its own and starts behaving in some contexts like a document, it aquires several methods and properties - and though I at first thought I could simply fake it with a DOMParser I had to think again because...

}catch(e){ // DOMParser could not parse fragment, probably because of missing single root element. Workaround time..
var d=document.implementation.createDocument('', this.tagName, null), el=d.createElement('el');
//?? why did I use this.tagName there?
el.innerHTML=s;
for(var i=0 ; i<el.childNodes.length;i++){
d.appendChild(el.childNodes[i].cloneNode(true));
}
}

Yes, they are not always playing with well-formed XML fragments. Oh well, we'll pull out good-old-tagsoup-parsing .innerHTML and eat their strings anyway. Then we move on and fill in some other required bits and pieces of IE's XML DOM. Right now I'm not sure if all the stuff in this block is required, but there it is.. I'll be the first to admit that both code and comments are evidence of the somewhat chaotic process of late-night patching..

// faking IE-style XML element DOM - separate documents with documentElement within the main doc's DOM
this.documentElement=d.documentElement||d.firstChild; 
//?? firstChild is probably leftover from earlier versions using documentFragment?
this.XMLDocument=d;
// address book loading checks .parseError.errorCode
this.XMLDocument.parseError={ 'errorCode':0 };
return d;
}

And then is a peculiar mystery, who would ever need a function called isSameNode when == would presumably do the job?

// some method called isSameNode is called. Not sure where it comes from but simple enough to fake..
Element.prototype.isSameNode = function(n){
return n===this;
}

Here we go, more delicacies from IE's internals: the handy XPath method selectSingleNode. I have quite some reservations against loadXML and the other XML DOM stuff above, but selectSingleNode should be written into a standard as soon as possible because document.evaluate needs too many arguments for lazy JS coders and the returned object is too fiddly too.

// selectSingleNode support
var realSelectSingleNode=function( expr, resolver ){
var result=(this.ownerDocument?this.ownerDocument:this).evaluate( expr+'[1]', this, resolver, XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,null );
return ( result && result.snapshotLength ) ? result.snapshotItem(0) : null;
}
Node.prototype.selectSingleNode = function (expr, resolver)
{ 
  if (!resolver)
    if (this.nodeType == Node.DOCUMENT_NODE){
      resolver = document.createNSResolver (this.documentElement);
    }else if(this.nodeType == Node.ELEMENT_NODE && this.ownerDocument && this.ownerDocument.documentElement ){
      resolver = document.createNSResolver (this.ownerDocument.documentElement);
    }else{
resolver = document.createNSResolver (this);
    }
  return realSelectSingleNode.apply (this, [expr, resolver]);
}

Now, up till now the patching has been quite ordered. A patch is a bit of a kludge anyway, but so far it's been a nice kludge. Here come the problems that were too ugly for a nice kludge. Certain issues just required search and replace operations on the script source code, nothing else to do about them - and because Yahoo source is compressed and variable names random, search and replace must take that into account and go for seriously complicated and ugly regular expressions.

opera.addEventListener('BeforeScript', function(e){
// This is the riskiest patch
// Fixing typo: missing ' after attribute value
e.element.text=e.element.text.replace( /\):\(">"\)\),/, "):(\"'>\"))," );
e.element.text=e.element.text.replace( / id='_test_add_folder>/, " id='_test_add_folder'>" );

Yep, twice in their source code they say things like <tag class="foo> omitting the closing quote. That caused Opera to parse it as a text node instead of an element, meaning source code would appear here and there in the interface. Oops.

// WebForms2 problem: button attribute "action" is a URL in WF2
e.element.text=e.element.text.replace( /\.(action\b)/g, ".js$1" );

Specs and implementations collide again: if you set input.action to a value like 'markAsSpam' the WF2 spec means it will be resolved as a URL, so when the script reads it again it will see 'http://mail.yahoo.com/dc/markAsSpam' which is not at all what it expected.

// send button not working - attribute nodes must be in the document they will be used
e.element.text=e.element.text.replace( /(\w)\.setAttributeNode\((\w)\)/, "$1.setAttributeNode($1.ownerDocument.importNode($2, true))" );

Hm, is Firefox sloppy with exceptions on cross-document node usage again?

// workaround for getting the documentElement.xml  markup
e.element.text=e.element.text.replace( /(([\w\.]*)documentElement).xml/g, "(document.implementation.createLSSerializer()).writeToString($1)" );

IE's XML DOM rides again. Elements over there have an .xml property which is basically the equivalent of .innerHTML for an HTML element, showing the inner serialised markup.

I didn't take a long and hard look at how Y!Mail used it but some of that seemed very weird. I had the impression that they read .xml of the contacts list only to pass it around as a string and use loadXML later on. Why would they serialize markup just to parse it right into a DOM tree again? Oh well, there is probably some complex reason..

// for...in on objects run into our faked __defineGetter__ and __defineSetter__
// we try to add an exception to any for...in loops
e.element.text=e.element.text.replace( /(for\((var |)(\w*) in \w*\)\{)/g, "$1if($3.match(/^__define/))continue;" );

It turns out "fake it until you make it" wasn't such a good idea after all. The site called our bluff with code like

function foo(obj){
for( p in obj )return false; return true
}

var bar = {}; if(!foo(bar)) return;

and what exactly they meant by that I don't know either, except to check that a newly created object really REALLY had no properties. Huh?

// To: / CC: autocomplete fails
// we support IE's TEXTAREA.createTextRange but unfortunately not its boundingLeft property. Improving object detection..
if(e.element.src&&e.element.src.match(/ac\.js$/))e.element.text=e.element.text.replace( /if \( editCtrl\.createTextRange \)/, "if ( editCtrl.createTextRange  && editCtrl.createTextRange().boundingLeft )" );

This is a typical trap of piecemeal implementation of something: we support whatever of IE's stuff was deemed important to get some plaintext formatting JavaScript to work with 8.x. .boundingLeft wasn't on the list back then. Sorry. Look a bit harder when you look for something.

// Preferences not read correctly from XML attributes
// IE has an attribute node .text property. .nodeValue will work in Opera..
e.element.text=e.element.text.replace( /\.selectNodes\((\w*)\);\s*\}return\((\w*)\.length\)\?(\w*)\[0\]\.text:/g, ".selectNodes($1);}return($2.length)?$2[0].nodeValue:" );

IE again. text alias nodeValue, enough said.

// We throw an unwanted exception if both arguments to insertBefore are the same node
e.element.text=e.element.text.replace( /var (\w*)=(\w*)\?(\w*)\.nextSibling:(\w*)\.firstChild;\s*(\w*)\.insertBefore\((\w*),(\w*)\);/, "var $1=$2?$2.nextSibling:$4.firstChild;if($6!=$1) $4.insertBefore($6,$1);" );

Now this is plainly a bug. The DOM spec says we should throw an error if the node you insert is a parent of the reference child, but we also did so if the inserted node was the reference child itself.

The code doesn't make sense, mind you... Why do you want to replace an element with itself?

// Opera 9.00 and 9.01 has a bug that means createContextualFragment on table elements is unreliable
// easily the worst patch.. but then it works around a really tricky bug..
if( navigator.userAgent.indexOf('9.01')>-1 || navigator.userAgent.indexOf('9.00')>-1 ){ 
// UA detection to target specific bug in specific version is OK
e.element.text=e.element.text.replace( /(\b(\w*)\.selectNodeContents\((\w*)\);\s*var (\w*)=(\w*).createContextualFragment\((\w*)\))/, "if($3.tagName=='TBODY'||$3.tagName=='TR'){ $2.createContextualFragment=function(s){var n=s.match(/<(\\w*)/)[1]; var e=document.createElement('div');e.innerHTML='<table><tbody>'+s+'</tbody></table>';return e.getElementsByTagName(n)[0];  } }$1" ); 

Just as ugly as it looks, just an attempt to make the code work in 9.01.
Imagine if Yahoo!Mail blocked us until they one day decided that it was necessarily to start working on Opera compatibility? If we never had gotten to test their system, they would probably have to add such an ugly workaround to their application to get around this bug. Developers everywhere: please, don't sniff, just leave more of the burden of compatibility on the UA's table (and listen to feedback!).

Hey, we're done with the replacements! It wasn't pretty, and I look forward to deleting one by one while things are fixed on either side. That will also give us a nice performance lift. Their scripts are huge. At some point I did some profiling of the above replace calls and found that up to half of the time it took Opera to load Y!Mail was spent applying the above patches.

}
}, false)

// No scrollbars appear for message list..
// uses an "overflow" CSS property to control scrollbars. e.overflow="-moz-scrollbars-vertical", and some odd clipping as well..
document.addEventListener( 'load', function(){ setTimeout( function(){ var divs=document.getElementsByTagName('div');for(var i=0,div;div=divs[i];i++)if(div.className&&div.className.indexOf('fakeScrollBar')>-1){div.style.overflow='auto';div.style.clip='auto';}},500);}, false );

Yes. That problem. It uses some CSS I still haven't fully understood.. I think they wanted to show an element with a scroll bar but clip the whole element away so only the scroll bar would be visible.

// redraw problem hides To: field in compose screen
document.addEventListener( 'load', 
function(){ if(top.document.frames['newmessage']){
setTimeout( function(){try{top.document.frames['newmessage'].document.body.className+=' ';}catch(e){}},1000);
}}, true);

This is another tricky one, it has to do with timing and I still haven't quite captured the sequence of events. Basically the "To" and "Subject" fields in the compose screen disappear until you click "Show BCC".

// sluggish performance due to unintended event capture
(function(ael){ 
window.addEventListener = function(type, func, capture){ ael.call(window, type, func, false); }
})(window.addEventListener);

..and just to top if off, they had to capture events by mistake. Of course. That's from the curriculum of "How to code Opera-incompatible websites 101".

opera.postError( 'Yahoo mail patched' );
}
}

Yippee! We did it!

Now, this patch is (repeat after me) in beta! There are problems that I'm aware of but haven't fixed, and there are problems that I'm not aware of and haven't fixed. And while I was working on this stuff, code changes in Yahoo mail would break things again every few days (and even break differently on the U.S. and the U.K. sites!). So, hurry up and try it while you have a chance! I'll try to keep the patch maintained, and we sure hope that we'll get all the issues sorted out from either side for a fast, friendly, responsive experience - somewhere in the future..

Just a morsel of technical documentation..

Opera's Java implementation will now support HTTPUrlConnection.setRequestProperty but for security reasons (to prevent so called "request smuggling attacks") it will not permit sending several headers. These headers include "Host", "Content-length" and "Transfer-Encoding".

This is the same security policy we apply to the XMLHttpRequest object and I believe the Flash plugin does the same thing for its ActionScript implementation.

If I remember things correctly this January was my fifth anniversary working for Opera.

Being CCed on 2.3% of the bugs in the bug tracker sure has taught me a lot since back then

Back then when we added the browser.js feature I heard some sceptical voices saying that fixing broken sites automatically was a bad idea because then the site had no incentive for fixing it themselves. The risk was that the web might become even more fragmented, with even worse examples of incompatible code because Opera would automagically fix things and cloak the faults of the webmasters.

Now call me an optimist, but we have about half a year's experience with browser.js and I'm seeing evidence of the opposite. Three good examples are allmusic.com, shockwave.com and atomfilms.com - they all had long-standing issues with Opera, they were patched successfully with browser.js and a few months after the patch, each site was fixed by the webmaster!

So perhaps, perhaps site patching does exactly what we hoped: increases Opera's ranking in site statistics by making previously unusable sites available to Opera users, thereby making webmasters more concerned about Opera compatibility (because such decisions are often based on browser stats) and eventually creating a more compatible web.

Of course it also helps that we always contact the website before or while we patch it.

It is no accident that browser.js is a simple text file written in readable and reasonably well commented JavaScript and that it always outputs some text in the JavaScript console when it does something. We could have done things differently, we considered pre-compiling the script somehow for performance - but in the end, it was most important to keep the whole feature as open for inspection as possible. And that pays off: we hear from web developers who sit down and read through the section of browser.js that is used on their website, for to-the-point, updated information about where in their site there are problems and scope for improvements. Thus browser.js itself becomes a way of communicating directly to the web developers we need to reach!

Hey, some of the fixes in browser.js can even be cut and pasted into the site to solve the problem

Yes, I think site patching works - and every patch I can remove from browser.js is a vote for that conclusion.