Closed Bug 1569102 Opened 5 years ago Closed 5 years ago

xmldocument load type error

Categories

(Core :: XML, defect)

68 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
relnote-firefox --- 69+
firefox-esr60 --- unaffected
firefox-esr68 69+ fixed
firefox68 + wontfix
firefox69 --- wontfix
firefox70 --- wontfix

People

(Reporter: vishal.jjit, Assigned: twisniewski)

Details

(Keywords: regression, site-compat)

Attachments

(2 files)

Attached file Resources.Browser.Form.js (deleted) —

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

We have checked with Firefox developer edition, in which object of xmldocument throwing error of Type error

Actual results:

We are using Visual web GUI framework to build our web application which is working fine till all earlier versions upto 67. Now it is showing Type error while loading the main page.

I see this in the code:

var s1051=document.implementation.createDocument("","",null);s1051.async=false;s1051.load(s788);

So this is likely because support for DOM3 load and async were removed in bug 332175, as explained here.

I'm not 100% sure if the code in question is something provided by Visual Web GUI or your own application? (It seems to be a library provided by Visual Web GUI, correct?)

Hi @Thomas Wisniewski,
What should be the status of this issue?should we need extra data from the reporter?
Therefore, I will set a component if isn't the proper one please fell free to change it.
Thanks.

Flags: needinfo?(twisniewski)
Component: Untriaged → XML
Product: Firefox → Core

Hi Thomas Wisniewski,

Greetings for the day....

Yes, the library is provided by Visual web GUI.

Thanks, Vishal. Could you also confirm which Visual Web GUI you are using and which version? (I'm guessing it's the one by Gizmox?)

Flags: needinfo?(twisniewski) → needinfo?(vishal.jjit)

Hi Thomas Wisniewski,

Yes, it is from Gizmox team and having express edition for .net 3.5.
The compiled installable is "VWG_Express_Studio_6.4.0_Release_e_NET3.5"

Flags: needinfo?(vishal.jjit)

Thanks! Ehsan, Adam, how do you feel about this one? Based on the note I'm seeing under the menu at [1], it seems as though Gizmox no longer support Visual WebGUI, and their last release was a version 10 with a free license, but I cannot access the URL to their downloads page ([2]).

My guess is that others sites may also be using this old library, so perhaps we ought to consider a webcompat intervention?

[1] https://www.componentsource.com/product/visual-webgui
[2] http://visualwebgui.com/visualwebgui/downloads

Flags: needinfo?(ehsan)
Flags: needinfo?(astevenson)

Thanks Tom. I assume that this is impacting some legacy sites, potentially in the Enterprise environment where we can't access. Not knowing which domains this is hosted on isn't great.

Mkaply - what's your take on usage of this in Enterprise?

Flags: needinfo?(astevenson) → needinfo?(mozilla)

I see in the JS code attached in comment 0 that the usage of XMLDocument.load() is gated based on a if (s722) condition. Earlier in that file we have:

var s721 =navigator.userAgent;/* ... */;var s722 =s721.indexOf("WebKit")!=-1;

So the generated code here gives us the wrong code because WebKit cannot be found in navigator.userAgent. :-(

Looking at the generated sources, it looks to me that the generated pages from the Visual WebGUI tool have file names following this pattern: Resources.Gizmox.WebGUI.*.htm.wgx. Is that right?

Perhaps we could use this file pattern to create a web-compat intervention to change our UA string for URLs like that?

Perhaps we could use this file pattern to create a web-compat intervention to change our UA string for URLs like that?

The issue is that overriding the UA might cause other issues, and we couldn't test it broadly. And looking at the script, it's checking s722 in many places, and it's unclear if some of those should be left as-is for some reason.

Another option would be for us to simply replace the string function sb428(s788,s88){if(s722){ with function sb428(s788,s88){if(true){ as we detect this specific script, but who knows if the minified variable names remain the same from version to version of the script (or which versions are affected).

Perhaps a stabler option would be to add a simple xmldoc.load polyfill to the start of any scripts we know are likely candidates for using XMLdoc.load that we want to temporarily intervene for; maybe something like this:

if (!XMLDocument.prototype.load) XMLDocument.prototype.load = function(url) {
  var x = new XMLHttpRequest();
  x.open("GET", url, false);
  x.send();
  this.appendChild(this.adoptNode(x.responseXML));
}

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

Perhaps we could use this file pattern to create a web-compat intervention to change our UA string for URLs like that?

The issue is that overriding the UA might cause other issues, and we couldn't test it broadly.

Yes, that's very true.

And looking at the script, it's checking s722 in many places, and it's unclear if some of those should be left as-is for some reason.

I do share your concern here.

Another option would be for us to simply replace the string function sb428(s788,s88){if(s722){ with function sb428(s788,s88){if(true){ as we detect this specific script, but who knows if the minified variable names remain the same from version to version of the script (or which versions are affected).

I suspect that this is compiled down from some higher level representation, and I am almost certain that the variable names aren't to be counted on.

Perhaps a stabler option would be to add a simple xmldoc.load polyfill to the start of any scripts we know are likely candidates for using XMLdoc.load that we want to temporarily intervene for; maybe something like this:

if (!XMLDocument.prototype.load) XMLDocument.prototype.load = function(url) {
  var x = new XMLHttpRequest();
  x.open("GET", url, false);
  x.send();
  this.appendChild(this.adoptNode(x.responseXML));
}

Yes, something like this may be a better option here.

Flags: needinfo?(ehsan)
Flags: needinfo?(mozilla)

Hi Julien, Adam, I am going to update the tracking/status flags on this bug. I am told that there is a webcompat impact here on release68 users. The fix would be a good one to ride-along in 68.0.2. Julien to make a final decision. Thanks!

Flags: needinfo?(jcristau)
Flags: needinfo?(astevenson)

re-enable XMLDocument.async and XMLDocument.load for webcompat on the ESR

Comment on attachment 9084114 [details]
Bug 1569102 - re-enable XMLDocument.async and XMLDocument.load for webcompat on the ESR; r?ehsan

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch re-enables the non-standard features XMLDocument.load and async for web compatibility's sake. It is only intended to land on the ESR branch, so users have a version of Firefox to fall back on which supports those features by default for a while longer (it is not intended to land on other branches).
  • User impact if declined: Users will have to create about:compat preferences manually in order to work around the webcompat issue (assuming they know it is the problem).
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Not risky. It simply toggles two preferences which users can toggle themselves.
  • String or UUID changes made by this patch:
Attachment #9084114 - Flags: approval-mozilla-esr68?

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Keywords: regression
Assignee: nobody → twisniewski
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment on attachment 9084114 [details]
Bug 1569102 - re-enable XMLDocument.async and XMLDocument.load for webcompat on the ESR; r?ehsan

Re-enables XMLDocument.async and XMLDocument.load on ESR to maintain better legacy compatibility. Approved for 68.1esr.

Attachment #9084114 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Flags: needinfo?(astevenson)

Do we need to track this for the next ESR too?

Flags: needinfo?(ehsan)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #17)

Do we need to track this for the next ESR too?

In a discussion with Adam and others from the webcompat team around a month ago we decided that we're only going to offer this workaround for the current ESR.

Flags: needinfo?(ehsan)

How can we publicize this change?

(In reply to Mike Kaply [:mkaply] from comment #19)

How can we publicize this change?

I guess one option is to add it to the 68 release notes page.

Liz, do you know if we have a post-facto process for adding things to release notes, and do you think it would have any impact?

Flags: needinfo?(lhenry)

I think it should go into the 68.1esr release notes (https://www.mozilla.org/firefox/68.1.0/releasenotes/)
I'll note as "Re-enabled XMLDocument.async and XMLDocument.load to maintain better legacy compatibility" unless you want to suggest other wording. To publicize it, maybe a blog post?

We could also just email the Enterprise list. Mike, do you want to do that?

Flags: needinfo?(lhenry) → needinfo?(mozilla)

I defer to Mike, I think he probably has more experience around how to communicate these types of changes most effectively than I do. :-) Thanks, Liz!

Do we have a good summary of what's changing?

Flags: needinfo?(mozilla)

XMLDocument.load and XMLDocument.async were removed in Firefox 68. We added them back to ESR 68.1 as a compatibility fix. We have no plans to keep them in the next ESR (75?).

Does that help?

And they don't exist in any other browser, right? We were the last holdout?

(In reply to Mike Kaply [:mkaply] from comment #26)

And they don't exist in any other browser, right?

Correct.

We were the last holdout?

These were always non-standard properties that Gecko supported and no other browser ever did. This comes from before when XHR existed, back then it could be used to fetch remote XML documents but was then superseded by XHR.

https://www.fxsitecompat.dev/en-CA/docs/2019/xmldocument-load-and-xmldocument-async-have-been-removed/ has a nicer write-up than my poor attempt here. :-)

Finally emailed the list.

Updated the compatibility note to say the change has been reverted in ESR 68.

Keywords: site-compat
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: