Mapbox-gl-native: [ios] Switching between label locales can make “null” labels appear

[friedbunny]

Mapbox SDK version: ios-v4.0.0-rc.1

Steps to trigger behavior

1. Use -[MGLStyle localizeLabelsIntoLocale:] to set a locale — e.g., nil or [NSLocale localeWithLocaleIdentifier:@"mul"].
2. Later, use -[MGLStyle localizeLabelsIntoLocale:] to set a different label locale.

Expected behavior

All labels will be localized into the desired locale. If no label should exist, none will exist.

Actual behavior

After switching to the second locale, some map features that shouldn’t be labeled gain “null” labels.

_This road at Tokyo Station._

Workaround

Call -[MGLMapView reloadStyle:] before switching locales, then set the new locale in -mapView:didFinishLoadingStyle:.

/cc @1ec5

[1ec5]

These ways lack name tags in OpenStreetMap and therefore lack name properties in the Streets source. However, I can’t reproduce the issue in macosapp – these roads continue to have no label. Is there a particular sequence of locale codes that leads to this issue?

[friedbunny]

Is there a particular sequence of locale codes that leads to this issue?

I saw this in treble when switching either way between nil (with a German system locale) and mul.

[1ec5]

The issue reproduces when using en in the second step, but not when using any other locale code, such as es. The following code path does allow for the return value of +[MGLVectorTileSource(Private) preferredMapboxStreetsLanguageForPreferences:] to be nil, but the relevant call site should already guard against that:

https://github.com/mapbox/mapbox-gl-native/blob/a6c02adeb6938e97aa6c78c67c0738bd5fda6bdc/platform/darwin/src/MGLVectorTileSource.mm#L123-L125
https://github.com/mapbox/mapbox-gl-native/blob/a6c02adeb6938e97aa6c78c67c0738bd5fda6bdc/platform/darwin/src/NSExpression+MGLAdditions.mm#L1420-L1423

[1ec5]

The layer in question is road-label-large in the Streets v10 style, which starts out as "text-field": "{name_en}", which the text getter translates to the expression name_en. Localizing to nil resolves to an expression like name_es, which translates to the JSON ["get", "name_es"]. But the text getter now returns CAST(name_es, "NSString").

Then, localizing to en produces an expression like CAST(name_en, "NSString"), which translates to ["to-string", ["get", "name_en"]]. The line feature lacks a name_en property, so that resolves to ["to-string", null] or "null".

[1ec5]

I’ve confirmed that a to-string expression is coming out of the getter _before_ replacing tokens with key paths here:

https://github.com/mapbox/mapbox-gl-native/blob/a6c02adeb6938e97aa6c78c67c0738bd5fda6bdc/platform/darwin/src/MGLSymbolStyleLayer.mm#L583

So it’s very likely that this cast is being introduced in mbgl.

/cc @anandthakker

[anandthakker]

which starts out as "text-field": "{name_en}", which the text getter translates to the expression name_en

I think this is probably the best place to fix this: can we translate "{name_en}" to (the NSExpression equivalent of) ["coalesce", ["get", "name_en"], ""]?

[1ec5]

Yes, that would be a possibility. However, more generally, it might be unexpected to a developer that ["concat", ["get", "name_en"], " (", ["get", "name"], ")"] would yield “null (null)” for features like the ones in the screenshot above. We may want to document the need to always coalesce any string-typed key path with an empty string, just as we’re documenting the need to cast key paths in predicates.

/cc @jmkiley @captainbarbosa @nickidlugash

[anandthakker]

Hm, good point. Would it be more appropriate for ["to-string", null] to yield "" instead of "null"?

[ChrisLoer]

Is this the right ticket to track porting https://github.com/mapbox/mapbox-gl-js/pull/6534 to gl-native? Since I don't see another ticket, I'll use this one for the native ignore until we make the corresponding native change to to-string.

[1ec5]

Yes, this ticket tracks a port of mapbox/mapbox-gl-js#6534.

[1ec5]

Fixed in #11904 on the release-boba branch for iOS map SDK v4.0.1 and macOS map SDK v0.7.1.