Bug 6907 – REGRESSION: United.com menus messed up due to document.all/MSIE sniff

• Component: JavaScriptCore
• Status: VERIFIED FIXED

[Rosyna]

If you visit http://www.united.com/ the entire left side of the page is unusable because of overlapping text/controls/tables.

[Rosyna]

Ah, actually seems to be the three JavaScript menus (Planning Travel, Travel Support, Mileage Plus) spilling into the content.

It is accompanied with these messages in the console:

[15475] http://a56.g.akamai.net/7/56/7207/149f8fe8f9724b/www.united.com/ual/asset/udm-control.js:TypeError: Undefined value
[15475] http://a56.g.akamai.net/7/56/7207/79cde4ae7eb8b9/www.united.com/ual/asset/udm-style.js:TypeError: Undefined value
[15475] http://a56.g.akamai.net/7/56/7207/426d2d78230e54/www.united.com/ual/asset/udm-dom.js:TypeError: Value undefined (result of expression um.addReceiver) is not object.
(event handler):Undefined value

[Darin Adler]

The menu implementation seems to be "UDM4" from www.udm4.com. Not sure exactly what's failing yet. We need the JavaScript debugger!

[David Kilzer (:ddkilzer)]

If the User Agent is changed to MSIE 6 via the Debug menu, the JS menu no longer "spills" into the page, but the drop-down menus don't appear when the mouse pointer hovers over the tabs.  Looks like different stylesheets are used depending on the platform (but not the browser):

<SCRIPT LANGUAGE="JavaScript">
<!--
if (navigator.appVersion.indexOf("Win") != -1) {
	if (document.layers){
		document.write("<LINK REL=stylesheet TYPE='text/css' HREF='/ual_style_pc_n.css'>");
	}
	// NN4+
	else if (document.all) {
	document.write("<LINK REL=stylesheet TYPE='text/css' HREF='/ual_style_pc_e.css'>");
    // MSIE4+
	}	
} else if (navigator.appVersion.indexOf("Mac") != -1) {
	document.write("<LINK REL=stylesheet TYPE='text/css' HREF='/ual_style_mac.css'>");
} else {
    // not a v 4 or +
}
//-->
</script>

[David Kilzer (:ddkilzer)]

Ignore the differences in stylesheets from Comment #3.  Those only change the size of fonts on the page.

The larger issue is that WebKit is becoming increasingly schizophrenic.  It is implementing increasingly more of an MSIE 6 personality, while continuing to maintain its Gecko (Firefox 1.x) personality.  "Old" detection code will soon (if it's not already) think that WebKit is MSIE 6 or Firefox 1.x depending on which it checks for first.  What's even more fun is that if there are multiple browser sniffing checks on a page, part of the JS code could assume it's on MSIE while another part of the code assumes it's on Firefox.

It looks like the "regressions" on this bug are (in part) due to new code paths being taken in the JavaScript on the web page due to WebKit behaving more like MSIE.  This means that to fix this bug, WebKit will have to implement more of MSIE's behavior.  More details to follow on the individual errors reported to the JavaScript Console.

[David Kilzer (:ddkilzer)]

Here are the JS errors reported:

TypeError: Undefined value
http://a56.g.akamai.net/7/56/7207/149f8fe8f9724b/www.united.com/ual/asset/udm-control.js
Line: 11

TypeError: Undefined value
http://a56.g.akamai.net/7/56/7207/79cde4ae7eb8b9/www.united.com/ual/asset/udm-style.js
Line: 11

TypeError: Value undefined (result of expression um.addReceiver) is not object.
http://a56.g.akamai.net/7/56/7207/426d2d78230e54/www.united.com/ual/asset/udm-dom.js
Line: 12

Undefined value
http://a56.g.akamai.net/7/56/7207/426d2d78230e54/www.united.com/ual/asset/udm-dom.js
Line: 11

Note that line number are pretty much meaningless since the JS is compacted into a single long line in each script.

[David Kilzer (:ddkilzer)]

TypeError: Undefined value
http://a56.g.akamai.net/7/56/7207/149f8fe8f9724b/www.united.com/ual/asset/udm-control.js
Line: 11

The first error is part of a JS check for Mac IE.

um.u=navigator.userAgent.toLowerCase();
// ...
um.wie=(um.d&&typeof um.m.all!=um.un&&typeof window.opera==um.un&&!um.k);
um.mie=(um.wie&&um.u.indexOf('mac')>0);
um.mx=(um.u.indexOf('tasman 0.9')>0);
if(um.mx){um.mie=1;}
um.omie=0;
if(um.mie){
  um.wie=0;
  um.iev=um.u;
  um.iev=um.iev.split('msie ');
  um.iev[1]=um.iev[1].split(';');  // um.iev[1] is undefined here
  um.iev=parseFloat(um.iev[1][0],10);
  um.omie=(um.iev<5.2);
}
// um.iev -- IE version number
// um.mie -- isMacIE?
// um.omie -- isOldMacIE? (before 5.2)
// um.wie -- isWinIE?

The only way I see to fix this error (other than evangelism) is to add an "MSIE X.Y" compatibility string to Safari's user agent, perhaps like this:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en) AppleWebKit/420+ (KHTML, like Gecko; MSIE 6.0) Safari/417.8

[David Kilzer (:ddkilzer)]

Created attachment 6093 [details]
Patch to add "MSIE 6.0" to user agent string

I don't expect this patch to be accepted, but it's here to prove a point:  WebKit is trying to act like MSIE in certain circumstances, so why not advertise it in the user agent string?

Note that this change prevents the drop-down menus from being painted on the web page itself, although there are still x/y (event?) offset issues when actually drawing the drop-down menus

Another approach would be to add "stealth" features to document.all and other commonly-used MSIE-specific DOM features so that browser sniffing code won't identify Safari+WebKit as MSIE.  This has already been implemented by Firefox.

http://www.mozillazine.org/talkback.html?article=5063

[Darin Adler]

Comment on attachment 6093 [details]
Patch to add "MSIE 6.0" to user agent string

This is the kind of thing that's going to have an effect on a vast number of websites. It's not clear how to decide whether this is safe or not. We can try it, but I'm worried that while it will fix problems on some sites, it will cause problems on other sites.

[Darin Adler]

We are already supposed to have "stealth" on our document.all. So I think the real issue is that our stealth document.all is failing.

[Darin Adler]

Comment on attachment 6093 [details]
Patch to add "MSIE 6.0" to user agent string

Lets pursue the "fix stealth document.all" path and come back to this if we decide that's the rigth thing to do. I believe Anders did the stealth document.all implementation and this initial version works for "==" but not for typeof.

[David Kilzer (:ddkilzer)]

Using a locally-saved copy, I have confirmed that making this expression true (implementing stealth mode for 'typeof document.all') will fix the united.com home page:

  (typeof document.all == 'undefined')

[David Kilzer (:ddkilzer)]

See Bug 6268 for the original "undetectable document.all" implementation.

[Anders Carlsson]

Created attachment 6136 [details]
fix

[Darin Adler]

Comment on attachment 6136 [details]
fix

Looks good, r=me.

Two comments: 1) lets get rid of the exec parameter to isEqualToNull; 2) lets think about giving that function a better name, since it's not just about being equal to null any more

[David Kilzer (:ddkilzer)]

Verified fixed as of r12491!

[Rosyna]

Me too! *hugs and kisses*

[Eric Seidel (no email)]

Removing Regression keyword from bugs already fixed.