fix(client): Enable loading different file types when running in parent mode without iframe

[chan1cyrus2]

Enable loading different file types when running in parent mode without iframe.

By default, the Karma middleware reads the included files and generates the corresponding HTML in string which eventually replaced the %SCRIPTS%. The client then later loads the iframe that embeds this context.html.

In #2542, I introduced a third option to run tests on lightweight browsers without iframe support. This is achieved by running test within the parent page and dynamically load all the files. I created a client_with_context.html which combines both client.html and context.html. The Karma middleware will concatenate all the included file paths, turns it to a long string with ',' separator and replace it with %SCRIPT_URL_ARRAY% in client_with_context.html. On the client side, it will read the %SCRIPT_URL_ARRAY% string and dynamically load the file one by one by creating script element. This is where the bug is introduced because I assume that all the included files are javascript, which actually covers most of the use cases in Karma.

Recently, we would like to run Karma for UI testing using a screenshot plugin and we see css files are actually not properly loaded. In this fix, instead of passing the files paths to client, we are passing the HTML element in string. The client reads the HTML in string and created the corresponding html element using DOMParser() and then it is appended to the DOM dynamically.

When I try out this new approach, one of the issue I see is script elements generated by DOMParser don't execute. Therefore, we have to use document.createElement for script elements.

[johnjbarton]

The change and the description are hard for me to connect. It seems like these changes could have much more impact and yet it is clear to me how file types play. Please expand upon the description to explain the changes.

[chan1cyrus2]

Updated the description with more detailed information. Please review. Thanks!

[johnjbarton]

Rather than "inanimate object does not like" can we say eg
// Escape characters with special roles in HTML that appear in the body of tags.

But I don't understand this. How can a DOM parser not support angle bracket parsing?

[chan1cyrus2]

Let's say we have only one file (bar/foo.js). We then construct this element that would like to add to the dom (<script type="text/javascript" src="/bar/foo.js" ></script>). When we embed this string into %SCRIPT_URL_ARRAY% in client_with_context.html. The html file will have a line look like this: window.karma.scriptUrls = ["<script type="text/javascript" src="/bar/foo.js" ></script>"]. When the browser parse the html file, they stop parsing once it reaches the character "<". They parse it and expect it as an open tag even though it is wrapped with double quotation. This happens on both Chrome and the lightweight browser I am using. Therefore I turn them to hex code which looks like this in the html: window.karma.scriptUrls = ["\x3Cscript type="text/javascript" src="/bar/foo.js" \x3E\x3C/script\x3E"]

[johnjbarton]

Wouldn't it be clearer to have the template be an array of objects:
[{type: "text/javascript", src: "/bar/foo.js" }, {...}] Then in this code we walk the object and create the script tags. So no angle brackets in the template means no reason to encode then replace the angle brackets?

[chan1cyrus2]

I also thought about this approach too. However, the script tags html strings are constructed case by case in the middleware.

if (fileType === 'css') {
              scriptTags.push(`<link type="text/css" href="${filePath}" rel="stylesheet">`)
            } else if (fileType === 'dom') {
              scriptTags.push(includedContent.get(file.path))
            } else if (fileType === 'html') {
              scriptTags.push(`<link href="${filePath}" rel="import">`)
            } else {
              const scriptType = (SCRIPT_TYPE[fileType] || 'text/javascript')
              const crossOriginAttribute = includeCrossOriginAttribute ? 'crossorigin="anonymous"' : ''
              scriptTags.push(`<script type="${scriptType}" src="${filePath}" ${crossOriginAttribute}></script>`)
            }
          }

If this changes in the future, we have to update the logic to construct and reconstruct the script tag object too. I think that actually adds another layer of complexity and maintenance. Therefore, I prefer just escaping and de-escaping the generated string here (which I think also have closest logic to the regular case).

[johnjbarton]

Wouldn't it be clearer to have the template be an array of objects:
[{type: "text/javascript", src: "/bar/foo.js" }, {...}] Then in this code we walk the object and create the script tags. So no angle brackets in the template means no reason to encode then replace the angle brackets?

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[chan1cyrus2]

Fix CLA

[chan1cyrus2]

@googlebot Fix CLA

[johnjbarton]

Is the different case significant? If yes please add a comment; if no please pick one.