finalPropName.js: emptyStyle undefined for document generated by in-browser XSLT

[sdh4]

Description

src/css/finalPropName.js defines a variable:

var cssPrefixes = [ "Webkit", "Moz", "ms" ],
	emptyStyle = document.createElement( "div" ).style,
	vendorProps = {};

that is used to do CSS property name translation. Under Chromium (tested v81) and Firefox (tested v76) if the document was loaded via an in-browser XSLT transformation then document.createElement( "div" ).style is undefined and attempts to use the emptyStyle variable in the remainder of the finalPropName code fail.

Recommended solution: Change emptyStyle line above to emptyStyle = document.createElementNS("http://www.w3.org/1999/xhtml", "div" ).style. This seems to work regardless of whether the document is XSLT or XML or not and in both Chromium and Firefox.

Link to test case

(Simple XSLT transformed XHTML. The transformation used is a no-op.)
http://thermal.cnde.iastate.edu/jqbug.xhtml (You will see an error in the browser console). Compare to http://thermal.cnde.iastate.edu/jqbug_fixed.xhtml (Difference is a patched jquery with above fix in place plus another patch based on current master branch)

The issue with .style being undefined is easy enough to demonstrate. Load either of the above test case URLs. In the Chromium/Chrome console enter: document.createElement( "div" ).style. It will show as undefined. Then try entering:
document.createElementNS("http://www.w3.org/1999/xhtml","div").style and it will give the correct object.

[mgol]

Thanks for the issue. We currently don't have any XHTML testing setup, not to mention including XSLT transformations. Is that the only place that requires modification? I'm a bit afraid of going this route if this requires changes in many more places, especially as long as we don't test it.

[mgol]

Looking at the patched jQuery that you provided, it also includes dropping one workaround for Android Browser that we still need in the 3.x line:
https://github.com/jquery/jquery/blob/3.5.1/src/css/curCSS.js#L33-L53
If such changes are required then this cannot be fixed before jQuery 4.0.

[sdh4]

Thanks for looking at this!

I didn't do any exhaustive searching or broader testcase for related problems but the link in the previous comment does highlight the one other case that I stumbled across that affected what I was doing. In that case the variable "style" may be null in which case the highlighted code will error out when extracting style.width

Any call to document.createElement() as opposed to document.createElementNS() has the potential to create an element with a null .style attribute. Likewise any access to the .style attribute of an element has the potential to be null.

So: Simply fixing the definition of emptyStyle in finalPropName.js will fix a lot and will stand on its own. But my testcase still doesn't work in that case (more discussion below).

A little bit of grepping through the source indicates that the total number of calls to createElement() and references to the .style attribute is reasonably small.

Here is the testcase with the emptyStyle definition fixed: http://thermal.cnde.iastate.edu/jqbug_halffixed.xhtml
It still fails (in 3.5 - stable) because support.pixelBoxStyles is undefined when referenced in curCSS.js (link in prior comment).

Why is pixelBoxStyles undefined?
Look at the code from around line 64 in css/support.js:

		container = document.createElement( "div" ),
		div = document.createElement( "div" );

	// Finish early in limited (non-browser) environments
	if ( !div.style ) {
		return;
	}

In the xslt/xhtml environment div.style from document.createElement() is null so the remainder of the function (which is what assigns support.pixelBoxStyles) never gets executed.

Realistically fixing all these things might be more than is desired in a stable release. Nevertheless the emptyStyle fix and perhaps modifying curCSS() to accommodate a null support.pixelBoxStyles are low risk.

Since the total number of calls to createDocument() or the element .style attribute is not all that large a broader fix does not look like it would be a lot of work.

Thanks!

[timmywil]

@sdh4 would you be willing to make a PR with your proposed fixes?

[sdh4]

Should I do just this focused issue (emptyStyle undefined) or attempt a broader fix? I must admit I'm not in much of a position to test the broader fix, but it shouldn't be difficult to write.

…

On Mon, 2020-08-31 at 09:46 -0700, Timmy Willison wrote: @sdh4 would you be willing to make a PR with your proposed fixes? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[mgol]

@sdh4 Isn't all of this fixable by changing all occurrences of document.createElement with proper document.createElementNS calls? Such a PR would help us in assessing how big the changes would need to be.

Ideally, we'd have this tested as no tests mean we may regress any moment, even before we manage to release the fix. Such testing would be done via Karma by adding another target to the karma section in Gruntfile. That separate target would need to override customContextFile & customDebugFile to point to something managed by a Node.js server that we already have defined in test/middleware-mockserver.js. You'd need to add an entry to mocks that would serve an in-browser XSLT document that would contain more or less what the files to which customContextFile & customDebugFile point to already have, just to trigger XSLT, nothing else. You can then access it via /test/data/mock.php?action=YOUR_NEW_KEY_IN_MOCKS with optional additional query parameters.

If you're willing to spend some time on adding such a test coverage, I'd be willing to help, guiding you through the process & helping if you get stuck. But, anyway, a good first step would be just a PR with source modifications for us to see how big this change would need to be.

[ygj6]

I looked at your example carefully and changed <xsl:output method="xml"/> to <xsl:output method="html"/> to solve the problem in the example;

In addition, I did the following test,

• Test scenario 1:
  The output document type specified in jqbug.xsl is xml (<xsl:output method="xml"/>), as in your example, since the tag in xml does not have style, so the result of document.createElement( "div" ).style is undefined.
  

• Test scenario 2:
  The output document type specified in jqbug.xsl is xml (<xsl:output method="html"/>)，since we specified the output document type of jqbug.xsl as html, and the html tag has style, the result of document.createElement( "div" ).style is normal.
  

• Test scenario 3:
  Modify the jquery source code, emptyStyle = document.createElement( "div" ).style to emptyStyle = document.createElementNS("http://www.w3.org/1999/xhtml", "div") .style, After testing, it is found that the code emptyStyle = document.createElementNS( "http://www.w3.org/1999/xhtml", "div" ).style will be executed by default after the introduction of jquery, the document type is displayed and modified to html, so everything normal.
  

I don't think jQuery needs to be modified, jQuery is primarily intended as a library for the HTML DOM.