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.

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

[Regression] Map stopped working in older Chrome versions #2529

Closed
SviMik opened this issue Feb 9, 2020 · 17 comments
Closed

[Regression] Map stopped working in older Chrome versions #2529

SviMik opened this issue Feb 9, 2020 · 17 comments

Comments

@SviMik
Copy link

SviMik commented Feb 9, 2020

I'm sure it worked a month or two ago on this PC, but now it doesn't, the map is just gray.

There are two errors in the console:
Uncaught TypeError: Cannot redefine property: name
Uncaught TypeError: Cannot read property 'body' of null

Sorry I can't be more of help, all the scripts are obfuscated.

@tomhughes
Copy link
Member

How "old" are we talking exactly?

@tomhughes
Copy link
Member

The fact that this affects the display of tiles suggests it is something in the leaflet library, but that hasn't changed in a long time.

I can't think of anything in our code that would be likely to have this effect and which has changed recently.

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 10, 2020

Uglifier was turned on again recently: d33cad4

@tomhughes
Copy link
Member

Yes but that had only been off for a short while - historically it has always been on.

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 10, 2020

Ok, then it's a question of which exact Chrome version the OP is using, and on which OS?

I would also try a full cache refresh to see if this helps.

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 10, 2020

I can reproduce this on Opera 12.16, Build 1860 on Ubuntu 18.04 x64.

Here's the non obfuscated local version based on 3bc4f8e

It points to some code inside of bootstrap. Maybe there's some bug, or the library is called in some unexpected ways.

[10.02.2020 23:03:31] JavaScript - http://localhost:3000/
Timeout thread: delay 4 ms
Uncaught exception: DOMException: NOT_SUPPORTED_ERR
Error thrown at line 3850, column 2 in <anonymous function: jQuery.readyException>() in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    throw error;
Error initially occurred at line 14709, column 4 in sanitizeHtml(unsafeHtml, whiteList, sanitizeFn) in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    var createdDocument = domParser.parseFromString(unsafeHtml, 'text/html');
called from line 15324, column 8 in _getConfig(config) in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    config.template = sanitizeHtml(config.template, config.whiteList, config.sanitizeFn);
called from line 14853, column 6 in Tooltip(element, config) in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    this.config = this._getConfig(config);
called from line 15389, column 10 in <anonymous function: Tooltip._jQueryInterface>() in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    data = new Tooltip(this, _config);
called via Function.prototype.call() from line 365, column 3 in <anonymous function: each>(obj, callback) in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    length = obj.length;
called from line 202, column 2 in <anonymous function: each>(callback) in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    return jQuery.each( this, callback );
called from line 15379, column 6 in _jQueryInterface(config) in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    return this.each(function () {
called from line 1472, column 8 in <anonymous function: control.onAdd>(layer, name, maxArea) in http://localhost:3000/assets/index.debug-a8e25a349a061b4c346f6a233b6490cb92f15d0614d0a20dfaae35d84ce94294.js:
    var item = $("<li>")
called from line 1523, column 6 in <anonymous function: control.onAdd>(map) in http://localhost:3000/assets/index.debug-a8e25a349a061b4c346f6a233b6490cb92f15d0614d0a20dfaae35d84ce94294.js:
    addOverlay(map.noteLayer, "notes", OSM.MAX_NOTE_REQUEST_AREA);
called from line 21726, column 2 in <anonymous function: addTo>(map) in http://localhost:3000/assets/application.debug-98541ea5446c89a752fb4e504c219d5b61bed92690ac65bf5bf0912a966f4423.js:
    var container = this._container = this.onAdd(map),

@tomhughes
Copy link
Member

So that is from bootstrap which is why it has started happening recently - probably not a huge amount we can do about it.

@tomhughes
Copy link
Member

Specifically I think it is the domParser.parseFromString invocation at https://github.com/twbs/bootstrap/blob/master/js/src/util/sanitizer.js#L104 that is throwing, presumably because the parseFromString method is not supported.

@tomhughes
Copy link
Member

Though https://caniuse.com/#feat=mdn-api_domparser_parsefromstring claims that Opera 12 should support it?

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 10, 2020

That's odd, yes. Here's a minimal version to reproduce the issue:

var unsafeHtml = '<div class="tooltip" role="tooltip"><div class="arrow"></div><div class="tooltip-inner"></div></div>'
var domParser = new window.DOMParser();
var createdDocument = domParser.parseFromString(unsafeHtml, 'text/html');

unsafeHtml happens to be the string at the place of the issue. It could be anything else, though. parseFromString will always fail with Unhandled DOMException: NOT_SUPPORTED_ERR

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 10, 2020

In any case that Opera version is quite dated. Let’s see which Chrome version the OP uses.

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 11, 2020

This seems related twbs/bootstrap-rubygem#171

In cases where we serve static text that doesn't depend on any user input, we might provide our own sanitizer function instead.

@HolgerJeromin
Copy link
Contributor

HolgerJeromin commented Feb 11, 2020

Reading https://developer.mozilla.org/en-US/docs/Web/API/DOMParser (and caniuse.com) I am pretty sure we are talking about an Chrome older than v31 (and Firefox <v12).
We are talking about DOMparser HTML support, not XML or SVG.

There is also a snipped (DOMParser HTML extension) for polyfilling this missing feature. Not sure if we should invest time into this security risk using this browser.
We are talking about 6-9 year old browsers.

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 11, 2020

#2511 sounds like the move to a newer bootstrap tooltip version is still planned, which may turn this issue obsolete.

@tomhughes
Copy link
Member

No I think that happened in e5c33c1 which is likely what triggered this.

@mmd-osm
Copy link
Contributor

mmd-osm commented Feb 11, 2020

Ok, so all planned tooltip related changes are already in place now, and we're not in some intermediate state where further changes would be expected. Tbh, this wasn't entirely clear to me when I read #2511. Probably that issue simply hasn't been updated yet in the meantime, and all is good.

@gravitystorm
Copy link
Collaborator

I am pretty sure we are talking about an Chrome older than v31 (and Firefox <v12).

Chrome 31 is from November 2013, and Firefox 12 is from April 2012. So I don't think we should spend any time on supporting these browsers.

We also see the same errors on the test suite, since we use PhantomJS and that doesn't implement this functionality either. But the solution there is to upgrade away from PhantomJS, and I'll open a separate issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants