1561036 - xmlDoc.onload doesn't get fired causing a difference in site appearance between Chrome and Firefox Nightly

getTypeBrowser: function (){
  /* Comp.Nav: Compatibilidad de navegadores */
  if(navigator.appName=='Netscape' && navigator.userAgent.indexOf('WebTV') == -1){
    if(parseInt(navigator.appVersion)==3 && (navigator.userAgent.indexOf('Opera') == -1 )){
      	this.brw = 'NN3';
      	this.subBrw = 'unknown';
      	this.cssEnabled = false;
    } else if( (parseInt(navigator.appVersion) >= 4)&& (navigator.userAgent.indexOf('Safari') == -1 )){
      	this.brw = parseInt(navigator.appVersion) == 4 ? 'NN4' : 'NN5';
      	if( navigator.userAgent.indexOf('like Gecko') != -1 ){
            this.subBrw = 'standard';
        } else {
            this.subBrw = 'unknown';
        }
    } else if (navigator.userAgent.indexOf('Safari/4') != -1  ) {
      	this.subBrw = 'S2';
      	this.brw = 'NN';
    } else if (navigator.userAgent.indexOf('Safari/') != -1  ) {
        this.subBrw = 'S3';
        this.brw = 'NN';
    }
  } else if(navigator.appName == 'Microsoft Internet Explorer'){
	    this.brw = (parseInt(navigator.appVersion) >= 4)?'IE5':'IE4';
    	this.subBrw = 'unknown';
  } else if(navigator.appName == 'Opera' && parseInt(navigator.appVersion) >= 5) {
	  	this.brw = 'O5';
    	this.subBrw = 'unknown';
  } if( this.brw.indexOf('NN')!=-1 ) {
 		this.brw = 'NN';
  } 
  return this.brw;
			
}

Funny things to note about this code:

The missing else keyword before the last if keyword causes NN3, NN4 and NN5 to be replaced with NN!
This code predates both Firefox and Chrome, it seems.

Anyways, when we run this mess of a UA sniffing code, we get this.brw == "NN"; this.subBrw == "unknwon";. When Chrome runs it, they get this.brw == "NN"; this.subBrw == "S3" (IOW, the code thinks Chrome is Safari 3 and we're some futuristic unknown Netscape based browser.)

Then we get to run this function:

importXML: function (fileXML,docRoot){
 	try{
		var xmlDoc = null;
		this.objXMLHttpRequest = null;
		var withoutActiveX = false;
		
		if (window.XMLHttpRequest && !window.ActiveXObject)
       	{
       		
       		if(this.brw == 'O5' || this.subBrw == 'S2' || this.subBrw == 'S3'|| this.subBrw == 'standard'){
       			
				this.objXMLHttpRequest = new XMLHttpRequest();
				if (this.objXMLHttpRequest.overrideMimeType)
					this.objXMLHttpRequest.overrideMimeType('application/xml');
				var jsDom = this;
				this.objXMLHttpRequest.onreadystatechange=function()
				{
					jsDom.loadAsyncResponse(jsDom,fileXML);
				};
				
				this.objXMLHttpRequest.open('GET',fileXML,false); 
				this.objXMLHttpRequest.send(null);
			}	
			else if (document.implementation && document.implementation.createDocument)
			{
				
				xmlDoc = this.getOut().implementation.createDocument('', 'doc', null);
				xmlDoc.async=false;
				xmlDoc.onload = this.onLoad;
			}	
		}
		else {
			if (window.ActiveXObject) {
				var sig = ["MSXML2.DOMDocument.5.0", "MSXML2.DOMDocument.4.0",
						  "MSXML2.DOMDocument.3.0", "MSXML2.DOMDocument",
						 "Microsoft.XmlDom"];
				for (var i=0; i < sig.length; i++) {
					try {
						xmlDoc = new ActiveXObject(sig[i]);
						break;
					 } catch (oE){}
				}
				if (xmlDoc==null) {	withoutActiveX =true; }
				if( !withoutActiveX && xmlDoc.async){
					xmlDoc.async=false;
					xmlDoc.setProperty('ForcedResync',false);
					xmlDoc.validateOnParse=false;
					xmlDoc.onreadystatechange = this.onLoad;
				} 
			}
			else withoutActiveX=true;

			if (withoutActiveX){
				var onLoadWithoutActiveX = function() {
					if (typeof document.all[fileXML] != 'undefined' && 
						document.all[fileXML].readyState == 'complete' ) {
						xmlDoc = document.all[fileXML].XMLDocument;
					}
				}
				var body = document.createElement("body");
				document.appendChild(body);
				var xml = document.createElement("xml");
				xml.onreadystatechange = onLoadWithoutActiveX;
				xml.id = fileXML;
				xml.src =  fileXML;
				xml.async = false;
				document.body.appendChild(xml);
				document.body.removeChild(xml);
				document.removeChild(body);
				this.xmlDocs[fileXML] = xmlDoc;
				this.onLoad();
				dom.setLangLiteral(xmlDoc);
			}
		}
		if ((this.objXMLHttpRequest==null && xmlDoc!=null) || (!withoutActiveX && xmlDoc != null)){
			this.xmlDocs[fileXML] = xmlDoc;
		    (typeof xmlDoc.async!='undefined')?xmlDoc.load(fileXML):xmlDoc.load(fileXML,this.getOut(),docRoot);
			dom.setLangLiteral(xmlDoc);
	    }
	}catch(e){
		dump(e.message+" importXML",0);
	}		
			
},

So we enter the buggy code path which relies on XMLDocument.onload as well as XMLDocument.async because of this.subBrw == "unknown", aka the "futuristic unknown version" part of being a Netscape based browser.

Has Regression Range: --- → yes

Has STR: --- → yes

status-firefox68: --- → affected

status-firefox69: --- → affected

tracking-firefox68: --- → ?

tracking-firefox69: --- → ?

Regressed by: 1328138

ExE Boss

Comment 2

•

5 years ago

That site is very likely also broken in modern IE.

(no longer active)

Comment 3

•

5 years ago

So in terms of fixing this bug, we have three different options that I can think of:

Reaching out to the web site asking them to fix their UA sniffing code. They can do that easily by changing this condition navigator.userAgent.indexOf('like Gecko') != -1 in getTypeBrowser to look like navigator.userAgent.indexOf('like Gecko') != -1 || navigator.userAgent.indexOf('Firefox/') != -1. From the looks of the code it seems quite old, so I'm not certain how reasonable it would be to expect the site to fix its bug here.
Adding a site-specific UA override. For example, setting the general.useragent.override.rolb.santanderbank.com pref to Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:69.0) like Gecko/20100101 Firefox/69.0 (which just injects like behind Gecko in our default UA string) fixes this bug by putting us in the same path in their UA sniffing code as the fix I suggested above.
Set the dom.xmldocument.async.enabled and dom.xmldocument.load.enabled prefs back to true on 68 and maybe 69, pending perhaps a response from the website. That's the least desirable option since it defeats our intention to remove these Firefox-only APIs.

Needinfoing some webcompat experts to solicit opinions on how we should approach this... Thanks, and sorry about the needinfo spam.

Flags: needinfo?(wisniewskit)

Flags: needinfo?(miket)

Flags: needinfo?(kdubost)

(no longer active)

Comment 4

•

5 years ago

(In reply to ExE Boss from comment #2)

That site is very likely also broken in modern IE.

Interestingly the labels all appear with the latest IE, Edge and Edgium on my machine...

Thomas Wisniewski [:twisniewski]

Comment 5

•

5 years ago

Based on our desired outcome, to me it sounds like we should do #1 and #2. That is, we should add a site patch while we try to get the site to fix their broken sniff, just in case they're still able and willing.

Flags: needinfo?(wisniewskit)

(no longer active)

Comment 6

•

5 years ago

(In reply to Thomas Wisniewski [:twisniewski] from comment #5)

Based on our desired outcome, to me it sounds like we should do #1 and #2. That is, we should add a site patch while we try to get the site to fix their broken sniff, just in case they're still able and willing.

Thanks!

Do we have any guidance about how to write a patch to do #2 given that the modifications to the UA string will be platform and version dependent?

For #1, do I need to file a separate bug? If yes, what would be the right component?

Thomas Wisniewski [:twisniewski]

Comment 7

•

5 years ago

We'll likely follow the same kind of procedure as we did for bug 1464106, filing a bug under Web Compatibility/Interventions, making a pull request against the addon's GH repository, and then merging into m-c from there.

But could you provide the specifics on what combinations of platform/etc should produce a certain UA string? Based on that we can figure out what alterations to the appropriate file are necessary, and how we'd like to proceed (in case we want to refactor the code a bit more instead of just adding a new chunk to that config).

Karl Dubost💡 :karlcow

Updated

•

5 years ago

Flags: needinfo?(kdubost)

Ksenia Berezina [:ksenia]

Reporter

Comment 8

•

5 years ago

I'll do #1 and try to reach out to the site.

(no longer active)

Updated

•

5 years ago

Depends on: 1563839

(no longer active)

Comment 9

•

5 years ago

(In reply to Thomas Wisniewski [:twisniewski] from comment #7)

We'll likely follow the same kind of procedure as we did for bug 1464106, filing a bug under Web Compatibility/Interventions, making a pull request against the addon's GH repository, and then merging into m-c from there.

But could you provide the specifics on what combinations of platform/etc should produce a certain UA string? Based on that we can figure out what alterations to the appropriate file are necessary, and how we'd like to proceed (in case we want to refactor the code a bit more instead of just adding a new chunk to that config).

Thanks, I filed bug 1563839 with what I believe should be the necessary information. If there is anything missing please needinfo me there?

Ryan VanderMeulen [:RyanVM]

Updated

•

5 years ago

status-firefox67: --- → unaffected

status-firefox-esr60: --- → unaffected

status-firefox-esr68: --- → affected

tracking-firefox68: ? → +

tracking-firefox69: ? → +

Mike Taylor [:miketaylr]

Comment 10

•

5 years ago

(In reply to :Ehsan Akhgari from comment #9)

(In reply to Thomas Wisniewski [:twisniewski] from comment #7)

We'll likely follow the same kind of procedure as we did for bug 1464106, filing a bug under Web Compatibility/Interventions, making a pull request against the addon's GH repository, and then merging into m-c from there.

But could you provide the specifics on what combinations of platform/etc should produce a certain UA string? Based on that we can figure out what alterations to the appropriate file are necessary, and how we'd like to proceed (in case we want to refactor the code a bit more instead of just adding a new chunk to that config).

Thanks, I filed bug 1563839 with what I believe should be the necessary information. If there is anything missing please needinfo me there?

SGTM

Flags: needinfo?(miket)

Mike Taylor [:miketaylr]

Comment 11

•

5 years ago

(In reply to Ksenia Berezina [:ksenia] from comment #8)

I'll do #1 and try to reach out to the site.

Ksenia sent me some LinkedIn contacts and I've messaged 3 individuals who work at Santander Bank N.A.

Keywords: webcompat:site-wait

(no longer active)

Updated

•

5 years ago

Flags: needinfo?(ehsan)

(no longer active)

Comment 12

•

4 years ago

Now that bug 1563839 is on autoland, what is the process for fixing the regression on beta and release? Normal code uplifts?

Mike Taylor [:miketaylr]

Comment 13

•

4 years ago

(In reply to :Ehsan Akhgari from comment #12)

Now that bug 1563839 is on autoland, what is the process for fixing the regression on beta and release? Normal code uplifts?

For beta, a simple uplift will do. For release, we either need to get PI to test the XPI and get relman to sign off for a balrog deploy, or we do a dot release ridealong. I'll ping relman and ask if there are upcoming dot releases planned -- that might be faster if something is in the pipeline.

(no longer active)

Comment 14

•

4 years ago

Thanks!

Since there is nothing more to be done here, I'm gonna resolve this bug as a duplicate of bug 1563839.

Status: NEW → RESOLVED

Closed: 4 years ago

Resolution: --- → DUPLICATE

Adam Stevenson [:adamopenweb]

Comment 15

•

4 years ago

I called the bank today and tried to get their IT department to look at this issue. Not sure how successful I was but they have all the right information now.

Ryan VanderMeulen [:RyanVM]

Updated

•

4 years ago

status-firefox69: affected → fixed

status-firefox70: --- → fixed

tracking-firefox70: --- → +

Julien Cristau [:jcristau]

Updated

•

4 years ago

status-firefox68: affected → fixed

Chema

Comment 16

•

4 years ago

:adamopenweb
Hi. My name is Jose Maria de Leon and I work for Portals in Santander UK IT division .

I have received an email from Santander US about this issue sent by you. First of all, thanks for informing us. Can you please clarify for me please? Has this issue being resolved in all releases but v68.0.0? Do we need to involve our IT Team? I'm asking the OLB team and they are not aware of this issue although I understand it will be raised soon. And from a Public site perspective www.santander.co.uk we haven't received any incident yet

Thanks

Flags: needinfo?(astevenson)

Chema

Comment 17

•

4 years ago

Hi again. I've just been informed about the issue in OLB for Corporate. Our CSA maintenance team is dealing with it. I understand that if Mozilla is fixing this for the newer versions this will not then conflict with our fixes...

Regards

Thomas Wisniewski [:twisniewski]

Comment 18

•

4 years ago

Chema, we are only temporarily working around the issue so your site continues to work, while your IT team has time to make a proper fix.

Basically, Firefox no longer supports two web features that are non-standard, as can be seen here: https://www.fxsitecompat.dev/en-CA/docs/2019/xmldocument-load-and-xmldocument-async-have-been-removed/

Your site is relying on at least one of those features when it detects Firefox, and it no longer has to do that (our temporary work-around just bypasses that check for Firefox on your site, causing it to fall back to using standard web features instead).

Flags: needinfo?(astevenson)

(no longer active)

Comment 19

•

4 years ago

Chema, thanks for getting back to us. Please note that in order to fix this issue, you can find the necessary information in comment 1. The problem we encountered is the old User-Agent sniffing code inside the function getTypeBrowser in https://rolb.santanderbank.com/Estatico/Globales/V74/Scripts/jsDomM.bjs which doesn't support parsing the Firefox User-Agent string (documented here). In order to work around the problem temporarily, we changed the User-Agent string that we send to your website to include like Gecko instead of Gecko in order to make it work with the current code in jsDomM.js.

Fixing this on your end requires updating the getTypeBrowser function to enable it to parse the Firefox User-Agent string correctly. One way to do that could be by looking for the "Gecko/" string inside User-Agent. Once this code has been updated on your end, we can remove the temporary work-around.

For testing purposes, you can see and control this temporary work-around in Firefox by typing about:compat in the browser address bar. Inside there you should see a corresponding entry for "rolb.santanderbank.com".

Hope this helps.

Chema

Comment 20

•

4 years ago

Thanks a lot Thoms and Ehsan

I have passed all this info to the labs in Madrid to resolve this for all versions of the CSS we use. One more question for our peace of mind. Has the release with the temporary fix already been released or when do you expect to release it?

Regards
Chema de Leon

Thomas Wisniewski [:twisniewski]

Comment 21

•

4 years ago

Chema, it appears that the temporary fix is due to land with the next dot-release of Firefox (68.0.2), based on what I'm seeing in bug 1567198. It likewise should end up in the next beta release, and is already in the nightly builds.

Chema

Comment 22

•

4 years ago

Thanks Thomas

Do you more or less know when this beta will be available to the public. I don't really know what do you mean by nightly bilds :). We are already applying our fixes in all applications affected but our internal process is unfortunately kind of slow...

Thomas Wisniewski [:twisniewski]

Comment 23

•

4 years ago

Sorry for the confusion! Basically, each release of Firefox starts off as nightly builds, then goes to beta, and then finally to release users.

We're hoping to get the fix our over the next week or two as part of Firefox 68.0.2, and I'll keep you updated.

However, I've just been told that we're not 100% sure if our temporary fix is covering all of the domains we need to cover. Could you confirm if these domains are all of the ones we might have to worry about?

If you or your engineers could verify this list, that could help speed things up (hopefully we are not missing anything!)

Chema

Comment 24

•

4 years ago

Thanks again for your quick response from the far wild west :)

Yes, I can confirm that those domains are correct

I will inform my UK teams to speed up the update of our software.

You have been very helpful.
Regards

Thomas Wisniewski [:twisniewski]

Comment 25

•

4 years ago

That's good to hear!

In that case, I have even better news: the fix is already being sent out to all Firefox users, even before the next dot-release. (But some users may not get it until the next dot-release).

Chema

Comment 26

•

4 years ago

Hiagain Thomas

With this news I yesterday asked our LCO to test again but unfortunately the situation was the same as before for everyone. So, its either we are part of those some users that will get the change with the dot releasae or the change hasn't really being released. I've alseo just tried this morning at home to and same situation.

Ryan VanderMeulen [:RyanVM]

Updated

•

4 years ago

status-firefox-esr68: affected → fixed