U4-11253 - New Chrome 66 trimStart function is overriding the Umbraco Core trimStart polyfill

Created by Janae Cram 19 Apr 2018, 17:20:18 Updated by Dennis Foster 12 Jul 2018, 08:26:09

Tags: Unscheduled

Is 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

Comments

Sebastiaan Janssen 19 Apr 2018, 18:03:40

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.


Sebastiaan Janssen 19 Apr 2018, 18:09:29

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.


Robert Copilau 20 Apr 2018, 07:38:54

No more 404 errors, merging!


Sebastiaan Janssen 20 Apr 2018, 08:46:33

Update: the color picker datatype in Umbraco also seems to be affected (see U4-11257).


Matt Brailsford 20 Apr 2018, 13:52:50

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.


Matt Brailsford 20 Apr 2018, 13:57:16

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.


Kevin Giszewski 20 Apr 2018, 16:52:43

Matt has a good suggestion.

Moving forward it might be worth a helper library instead of using prototyping.


Sebastiaan Janssen 21 Apr 2018, 06:58:45

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