mapbox / mapbox-gl-draw

Draw tools for mapbox-gl-js
https://www.mapbox.com/mapbox-gl-js/example/mapbox-gl-draw/
ISC License
942 stars 590 forks source link

IE 11 is not parsing valid GEOJSON when loading #666

Closed keyofj closed 4 years ago

keyofj commented 7 years ago

draw v19 mapboxgl v38

This only happens in IE 11, but on Windows 8 and Windows 10 alike.

Test case:

Go to http://geojsonlint.com/

Use the following geojson

{
    "type": "FeatureCollection",
    "features": [{
        "type": "Feature",
        "geometry": {
            "type": "Polygon",
            "coordinates": [
                [
                    [-82.6123, 39.6891],
                    [-82.6174, 39.6597],
                    [-82.5669, 39.6584],
                    [-82.5551, 39.6794],
                    [-82.5779, 39.6765],
                    [-82.6123, 39.6891]
                ]
            ]
        },
        "properties": {
            "id": "1497701069ffef972907dfea7b2f6aaf",
            "color": "#444444",
            "lineWidth": 2,
            "type": "boundary"
        }
    }]
}

It will throw the following error in IE 11:

image

This site in Chrome, FIrefox and Edge it works fine.

The reason for reporting this is that when loading a saved map that had used the polygon draw tool, this same thing happens, although I can verify that the GEOJSON is valid and correct that is being loaded into the drawing tool. It only fails in IE 11, but works perfect in Chrome and Firefox.

Have you seen this before?

I have tried various suggestions such as adding header declarations to force IE Edge rendering despite the DOCTYPE with no avail.

keyofj commented 7 years ago

@andrewharvey Further information: When loading the drawing API this fails in IE 11 but works in Chrome, Firefox and Edge.

var _drawingManager = new MapboxDraw({
    displayControlsDefault: false
});

var map = nmap.map.getMap();   // mapboxgl map
map.addControl(_drawingManager, "bottom-left");

var featureId = _drawingManager.add({
    "type": "FeatureCollection",
    "features": [{
        "type": "Feature",
        "geometry": {
            "type": "Polygon",
            "coordinates": [
                [
                    [-82.6123, 39.6891],
                    [-82.6174, 39.6597],
                    [-82.5669, 39.6584],
                    [-82.5551, 39.6794],
                    [-82.5779, 39.6765],
                    [-82.6123, 39.6891]
                ]
            ]
        },
        "properties": {
            "id": "1497701069ffef972907dfea7b2f6aaf",
            "color": "#444444",
            "lineWidth": 2,
            "type": "boundary"
        }
    }]
});

You can see it in action using this link: Test Map

AnnaStaley commented 7 years ago

@keyofj thank you for cutting this ticket! Looping in @mcwhittemore for further discussion.

mcwhittemore commented 7 years ago

Sadly I don't currently have access to an IE browser so I can't be much help here.

One thing I've seen before is the use of polyfills creating problems by changing how Object works.

mcwhittemore commented 7 years ago

Its possible that this is due to https://github.com/mapbox/geojsonhint as it looks like this is happening when the feature is being parsed by Draw.

keyofj commented 7 years ago

Yes I think so. When in IE 11 it appears to not recognize constructor.name and therefore fails the validation of the objects because it returns undefined.

Get Outlook for Androidhttps://aka.ms/ghei36


From: Matthew Chase Whittemore notifications@github.com Sent: Thursday, July 13, 2017 5:56:58 PM To: mapbox/mapbox-gl-draw Cc: Michael Clark; Mention Subject: Re: [mapbox/mapbox-gl-draw] IE 11 is not parsing valid GEOJSON when loading (#666)

Its possible that this is due to https://github.com/mapbox/geojsonhint as it looks like this is happening when the feature is being parsed by Draw.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/mapbox/mapbox-gl-draw/issues/666#issuecomment-315213512, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AHoyQKq4-yXNjts1jWgA6ACbKvyVpDzcks5sNpKpgaJpZM4OOehj.

mcwhittemore commented 7 years ago

Does https://www.mapbox.com/geojsonhint/ work in IE11 for you?

keyofj commented 7 years ago

@mcwhittemore When I copy and paste the above code into the mapbox geojsonhint page and click format, it does not throw any errors when using IE 11 on Windows 8 or Windows 10.

mcwhittemore commented 7 years ago

@keyofj - I've been unable to reproduce this bug. Can you provide a jsfiddle with the minimal functionality needed to make this happen per the issue template. Thanks!

keyofj commented 7 years ago

I will work on a jsfiddle Friday as I am out of the office on other business. Or sooner if possible. Thank you.

keyofj commented 7 years ago

@mcwhittemore Here is the jsfiddle link and a screenshot of what happens in IE 11. If you run this fiddle in Chrome or Firefox, it works fine.

jsfiddle

LINE 1529 of mapbox-gl-draw.js validation routines throws error: "properties" member should be an object, but is an undefined instead.

image

mcwhittemore commented 7 years ago

OK. I've confirmed that this is coming from this line but I'm not sure why geojsonhint has this error as part of Draw but not as part of its own website.

Also, stepping though the debugger, I can confirm that the the geojson has these properties.

Next steps here are to work with the geojsonhint project to figure out why this bug is happening and than update Draw when geojsonhint releases a fix.

juhanivl commented 6 years ago

I think I've found a solution and cause for this. When calling add() mapbox-gl-draw uses mapbox geojsonhint to validate GeoJSON. When validating the feature properties geojsonhint uses constructor.name property to check whether or not it's "Object" link

By "forcing" the feature properties constructor name to be "Object" I was able to pass GeoJSON features to draw. drawnShapeData.features[0].properties.constructor.name = "Object" draw.add(drawnShapeData);

Maybe geojsonhint could use some other way to validate feature's properties in order to avoid this error since constructor.name property is non-standard and clearly causing issues.

keyofj commented 6 years ago

Thanks @juhanivl I should have considered that as a work around for now. Great point!

snkashis commented 6 years ago

I'm still seeing this error coming from our IE11 users... could this be a possible workaround upstream in geojsonhint? https://stackoverflow.com/a/4333252/1238737

Erutan409 commented 6 years ago

@snkashis Same here. I just came across this gem/bug as well.

Erutan409 commented 6 years ago

@juhanivl Thank you. This indeed fixes the issue for me, too. Oh, IE...

stutrek commented 6 years ago

I have created a PR to fix this issue. https://github.com/mapbox/geojsonhint/pull/70

kkaefer commented 4 years ago

We're now using @mapbox/geojsonhint@3.0.0 which includes the upstream fix.