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

Glyph PBF request failure prevents entire map from loading #4563

Open
1ec5 opened this issue Aug 15, 2024 · 3 comments · May be fixed by #4564
Open

Glyph PBF request failure prevents entire map from loading #4563

1ec5 opened this issue Aug 15, 2024 · 3 comments · May be fixed by #4564
Labels
need more info Further information is requested

Comments

@1ec5
Copy link
Contributor

1ec5 commented Aug 15, 2024

If any character in a text-field causes GL JS to request a glyph PBF that’s unavailable for some reason, the entire map fails to load.

Example

In this example, the variable char is set to \uFFFC (). The specified fontstack lacks a glyph PBF at https://demotiles.maplibre.org/font/Open%20Sans%20Semibold/65280-65535.pbf. GL JS receives an HTTP 404 error and refuses to load the map at all, even unrelated features in other tiles.

A
A map with a gray background and a label that reads “ABC A ABC” A blank map

At the very least, GL JS should skip over the character but render the rest of the map. Ideally, though, it would fall back to local glyph rendering, since even a crude rendering of the glyph would be less confusing than no rendering.

Impact

One can imagine a denial of service scenario in which someone litters OpenStreetMap with hidden control characters that don’t render in a standard text renderer but cause GL JS to refuse to render any part of the map.

In #4550 (comment), I was asked to remove the artificial limitation that prevents GL JS from even trying to render non-CJK characters above U+FFFF. But this imposes a new requirement on the developer that the fontstack must provide new glyph PBFs for every 255-character range from U+FFFF to U+10FFFF. This is significant time commitment, given that some of the most popular glyph PBF generators don’t support codepoints beyond U+FFFF and there are fewer fonts to choose from. It’s also a lot of files to host, probably for little justification. If the developer doesn’t supply these new glyph PBFs, then upgrading to a release of GL JS that contains #4550 would expose their map to a nontrivial number of places and POIs in East Asia whose names would essentially crash the map.

A fallback would eliminate this burden, allowing the browser to render the text as a best effort. This is consistent with standard font fallback behavior in CSS-compliant browsers.

Previously reported as mapbox/mapbox-gl-js#9528 mapbox/mapbox-gl-js#9546.

@HarelM
Copy link
Collaborator

HarelM commented Aug 15, 2024

This is not a regression that was introduced as part of version 3 or 4, right?
Generally speaking, a fallback would be a better user experience, I would also reccomend a warnOnce so that a developer would know that a callback has been used if they want to fix the style (much like other style failures).

@HarelM HarelM added the need more info Further information is requested label Aug 15, 2024
@1ec5
Copy link
Contributor Author

1ec5 commented Aug 15, 2024

This apparently worked correctly in v0.3, judging from mapbox/mapbox-gl-js#662, but it’s been broken like this as long as I’ve used GL JS or gl-native, certainly since before the fork.

Developers have generally coped with this issue by generating comprehensive glyph PBFs for U+0000 through U+FFFF, filling in nonsense glyphs for anything the font lacks. However, this becomes much less feasible if we open up remote glyphs behind the BMP.

I agree that some sort of warning or error in the console would be a good idea, as long as it doesn’t interfere with tile loading. After all, the request failure could be due to the style referencing a nonexistent fontstack.

@1ec5
Copy link
Contributor Author

1ec5 commented Aug 15, 2024

Also reproduces in Mapbox GL JS v0.45.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more info Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants