Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid document write in single script linker loading #9814

Closed
wants to merge 1 commit into from
Closed

Avoid document write in single script linker loading #9814

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 1 addition & 20 deletions dev/core/src/com/google/gwt/core/linker/SingleScriptTemplate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,6 @@ function __MODULE_FUNC__() {
// base url.
//
function computeScriptBase() {
var thisScript
,markerId = "__gwt_marker___MODULE_NAME__"
,markerScript;

$doc.write('<script id="' + markerId + '"></script>');
markerScript = $doc.getElementById(markerId);

// Our script element is assumed to be the closest previous script element
// to the marker, so start at the marker and walk backwards until we find
// a script.
thisScript = markerScript && markerScript.previousSibling;
while (thisScript && thisScript.tagName != 'SCRIPT') {
thisScript = thisScript.previousSibling;
}

// Gets the part of a url up to and including the 'path' portion.
function getDirectoryOfFile(path) {
// Truncate starting at the first '?' or '#', whichever comes first.
Expand All @@ -119,6 +104,7 @@ function __MODULE_FUNC__() {
return (slashIndex >= 0) ? path.substring(0, slashIndex + 1) : '';
};

var thisScript = $doc.currentScript;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to caniuse.com currentScript is not supported by IE 11. So you can not use it for now unless a decision has been made to drop IE 11 in the next GWT release.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you OK to make the code try currentScript and if fails to try the old implementation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems reasonable to me - put an explicit note so that when IE 11 is finally truly dead we know that this can be simplified out?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you OK to make the code try currentScript and if fails to try the old implementation?

Yes that would be fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Mivr do you want to make this change so we can see about merging?

Copy link
Author

@Mivr Mivr May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niloc132 Yes, sorry for the delay I am just way too busy and warnings are not the highest priority

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep no problem, just checking in with various open PRs to see what we can land before trying to do a release. No rush, we've got other pending PRs, but this one is small enough we could ship without it if you don't get the chance to get to it.

if (thisScript && thisScript.src) {
// Compute our base url
base = getDirectoryOfFile(thisScript.src);
Expand Down Expand Up @@ -146,11 +132,6 @@ function __MODULE_FUNC__() {
img.src = base + 'clear.cache.gif';
base = getDirectoryOfFile(img.src);
}

if (markerScript) {
// remove the marker element
markerScript.parentNode.removeChild(markerScript);
}
}

// Called to slurp up all <meta> tags:
Expand Down