We have moved to GitHub Issues
You are viewing the read-only archive of Umbraco's issue tracker. To create new issues, please head over to GitHub Issues.
Make sure to read the blog posts announcing the move for more information.
Created by Janae Cram 19 Apr 2018, 17:20:18 Updated by Dennis Foster 12 Jul 2018, 08:26:09
Tags: UnscheduledIs duplicated by: U4-11257
Relates to: U4-11248
Today, I upgraded DocType Grid Editor for a client and discovered that there are a ton of 404 errors being returned. I posted [https://github.com/umco/umbraco-doc-type-grid-editor/issues/97 an issue] on Umco's DTGE GitHub project, and we discovered through our problem-solving that the issue has to do with the Chrome 66 release and the addition of the trimStart
function, which doesn't actually trim the start of a string, and is overriding Umbraco's js trimStart
polyfill extension [https://github.com/umbraco/Umbraco-CMS/blob/release-7.10.3/src/Umbraco.Web.UI.Client/lib/umbraco/Extensions.js#L90-L95 here].
To sum up, all of DTGE's API calls contained the tilde in the URL. The call that's stripping that out is here:
var url = umbRequestHelper.convertVirtualToAbsolutePath("~/umbraco/backoffice/DocTypeGridEditorApi/DocTypeGridEditorApi/GetContentTypes");
That is using the trimStart function being called [https://github.com/umbraco/Umbraco-CMS/blob/release-7.10.3/src/Umbraco.Web.UI.Client/src/common/services/umbrequesthelper.service.js#L30 here].
Some Chrome debug testing shows that using trimStart in the developer tools is doing absolutely nothing, as you can see in my attached image.
I already linked it, but for our thorough conversation, please check the DTGE issue: https://github.com/umco/umbraco-doc-type-grid-editor/issues/97 There was also a forum discussion with other people talking about how it was not just DTGE (which is part of how we hunted it down) that you can read through here: https://our.umbraco.org/projects/backoffice-extensions/doc-type-grid-editor/doc-type-grid-editor-feedback/91647-404-on-getcontenttypeicon-and-4931dtgepreview-1
trimStart is being used in a few places in Core ([https://github.com/umbraco/Umbraco-CMS/search?l=JavaScript&q=trimStart apparently 7]) that could have unexpected results, so I wanted to raise this as quickly as possible since it's likely to cause a lot of problems for Chrome users :)
=Manual patching / workaround=
A workaround for all affected sites is to update the file ~/Umbraco/lib/umbraco/Extensions.js
. The changes that need to be made are in the following two functions:
if (!String.prototype.trimStart) {
/** trims the start of the string*/
String.prototype.trimStart = function (str) {
if (this.startsWith(str)) {
return this.substring(str.length);
}
return this;
};
}
if (!String.prototype.trimEnd) {
/** trims the end of the string*/
String.prototype.trimEnd = function (str) {
if (this.endsWith(str)) {
return this.substring(0, this.length - str.length);
}
return this;
};
}
In order to make Umbraco work properly again, you need to remove the checks if (!String.prototype.trimStart)
and if (!String.prototype.trimEnd)
. The result should look like this:
/** trims the start of the string*/ String.prototype.trimStart = function (str) { if (this.startsWith(str)) { return this.substring(str.length); } return this; };
/** trims the end of the string*/
String.prototype.trimEnd = function (str) {
if (this.endsWith(str)) {
return this.substring(0, this.length - str.length);
}
return this;
};
After changing this file, you will also need to update ~/Config/ClientDependency.config
, increase the version number in there by one. This will recycle the site and clear browser cache. After that plugins that were broken (like Doc Type Grid Editor) should work again.
1 Attachments
PR: https://github.com/umbraco/Umbraco-CMS/pull/2597
This method was introduced in v7.0.0 so all v7 site could be affected See initial commit: https://github.com/umbraco/Umbraco-CMS/commit/dd3490a2ab26b753610d07ca5fffcdc645ef2b58#diff-6700cb6404c7d31435f572a58a38ce67
A workaround for all affected sites is to update the file ~/Umbraco/lib/umbraco/Extensions.js
. The changes that need to be made are in the following two functions:
if (!String.prototype.trimStart) {
/** trims the start of the string*/
String.prototype.trimStart = function (str) {
if (this.startsWith(str)) {
return this.substring(str.length);
}
return this;
};
}
if (!String.prototype.trimEnd) {
/** trims the end of the string*/
String.prototype.trimEnd = function (str) {
if (this.endsWith(str)) {
return this.substring(0, this.length - str.length);
}
return this;
};
}
In order to make Umbraco work properly again, you need to remove the checks if (!String.prototype.trimStart)
and if (!String.prototype.trimEnd)
. The result should look like this:
/** trims the start of the string*/ String.prototype.trimStart = function (str) { if (this.startsWith(str)) { return this.substring(str.length); } return this; };
/** trims the end of the string*/
String.prototype.trimEnd = function (str) {
if (this.endsWith(str)) {
return this.substring(0, this.length - str.length);
}
return this;
};
After changing this file, you will also need to update ~/Config/ClientDependency.config
, increase the version number in there by one. This will recycle the site and clear browser cache. After that plugins that were broken (like Doc Type Grid Editor) should work again.
So for the details:
We have always had a trimStart
and trimEnd
method that takes a parameter to trim specific characters at the beginning or the end of a string. Chrome 66 has implemented it's own method with the same name, it doesn't take a parameter though and ONLY trims whitespace. This means that the characters that we used to trim are now no longer trimmed as of Chrome 66.
The change in the PR overrules Chrome's implementation (and the implementation that any other browser has built in) so that it works like we expected again.
No more 404 errors, merging!
Update: the color picker datatype in Umbraco also seems to be affected (see U4-11257).
I'm wondering, given Umbraco's implementation requires a param of what is to be trimmed, wouldn't it be better to say "if it has a param, do our code, if it has no param, do the browsers default behaviour, if it has one"? At the moment you are blanket replacing the browsers implementation which could have adverse effects later on.
Or at least assume that if str
is undefined or empty that you are wanting to trim blank space thus ensuring default browser behaviour is replicated should anyone being relying on that in a pakage or something.
Matt has a good suggestion.
Moving forward it might be worth a helper library instead of using prototyping.
Absolutely, this needs to be fixed properly at a later point and we'll need to evaluate all of the methods in this Extensions.js file soon.
Priority: Major
Type: Bug
State: Fixed
Assignee:
Difficulty:
Category:
Backwards Compatible: True
Fix Submitted:
Affected versions: 7.0.0, 7.1.0, 7.0.1, 7.0.2, 7.0.3, 7.0.4, 7.1.1, 7.2.0, 7.1.2, 7.1.3, 7.1.4, 7.1.5, 7.3.0, 7.1.6, 7.1.7, 7.1.8, 7.1.9, 7.2.1, 7.2.2, 7.2.3, 7.2.4, 7.2.5, 7.2.6, 7.2.7, 7.4.0, 7.2.8, 7.3.1, 7.5.0, 7.6.0, 7.3.2, 7.3.3, 7.3.4, 7.4.1, 7.3.5, 7.3.6, 7.3.7, 7.3.8, 7.4.2, 7.4.3, 7.5.1, 7.5.2, 7.5.3, 7.5.4, 7.5.5, 7.5.6, 7.5.7, 7.5.8, 7.7.0, 7.5.9, 7.5.10, 7.5.11, 7.5.12, 7.5.13, 7.6.1, 7.5.14, 7.6.2, 7.6.3, 7.6.4, 7.6.5, 7.8.0, 7.6.6, 7.7.1, 7.6.7, 7.6.8, 7.7.2, 7.6.9, 7.7.3, 7.6.10, 7.7.4, 7.6.11, 7.7.5, 7.6.12, 7.7.6, 7.6.13, 7.7.7, 7.7.8, 7.9.0, 7.7.9, 7.7.10, 7.7.11, 7.8.1, 7.7.12, 7.7.13, 7.10.0, 7.9.1, 7.9.2, 7.9.3, 7.10.1, 7.10.2, 7.9.4, 7.10.3, 7.8.2, 7.9.5
Due in version: 7.10.4, 7.8.3, 7.9.6
Sprint: Sprint 83
Story Points:
Cycle: 9