mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.08k stars 2.21k forks source link

isPatternMissing can throw a TypeError under certain conditions #9518

Closed tylergaw closed 4 years ago

tylergaw commented 4 years ago

mapbox-gl-js version: 1.9.1

This issue showed up first in 1.9.0. It's not in versions 1.7.x and 1.8.x

browser:

Steps to Trigger Behavior

  1. Visit the public style or demo link below
  2. Open the developer console
  3. Zoom out less than z11
  4. Note the error in the console
Uncaught TypeError: Cannot read property 'toString' of null
    at go.isPatternMissing (painter.js:593)
    at Object.background (draw_background.js:28)
    at go.renderLayer (painter.js:490)
    at go.render (painter.js:435)
    at r._render (map.js:2206)
    at map.js:2330

Link to Demonstration

The issue occurs on the public style page as well as in other implementations. Both available here in case one is easier to use than the other.

Expected Behavior

Actual Behavior

Details

This is happening here https://github.com/mapbox/mapbox-gl-js/blob/844ea8dc0e62930f748c91e2930444edcb75eb4d/src/render/painter.js#L514

It's possible for image.from to be null. So the toString() invocation throws as expected.

Getting to the scenario is pretty specific. In a style, you have to have a layer with a background pattern that changes from None to any pattern. In the shared style above that is the background-texture layer. It looks like this:

[
  "step",
  ["zoom"],
  "",
  11,
  "background-texture"
]

From z0 to z11 There's no Pattern set. Then at z11 we apply the background-texture image as a pattern.

Workaround

To get around this for now we've updated the layer style to not toggle the pattern off/on. Instead, we always have the background-texture pattern applied and update the layer opacity to hide it.

opacity:

[
  "interpolate",
  ["linear"],
  ["zoom"],
  10,
  0,
  11,
  1
]

That works for now, but it seems like isPatternMissing needs a bit more guarding to protect against invoking toString on null objects.

karimnaaji commented 4 years ago

Thanks for reporting this. The regression was traced back to commit https://github.com/mapbox/mapbox-gl-js/commit/fa3616825411009284b99daae7f27368fad774af, specifically https://github.com/mapbox/mapbox-gl-js/commit/fa3616825411009284b99daae7f27368fad774af#diff-75421c871234afa60c44391b9c0ef05aR21.

cc @mourner