Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Automatic label localization should transform non-constant expressions #10713

Closed
1ec5 opened this issue Dec 16, 2017 · 11 comments
Closed

Automatic label localization should transform non-constant expressions #10713

1ec5 opened this issue Dec 16, 2017 · 11 comments
Assignees
Labels
iOS Mapbox Maps SDK for iOS localization Human language support and internationalization macOS Mapbox Maps SDK for macOS refactor

Comments

@1ec5
Copy link
Contributor

1ec5 commented Dec 16, 2017

MGLStyle.localizedLayersByIdentifier is a dictionary of dictionaries of MGLTextLanguage objects. The structure looks something like this:

[
    "water layer": [
        "{old_name}": MGLTextLanguage(textLanguage: "{old_name}", updatedTextField: "{new_name}")
    ],
    "road layer": [
        "{old_name}": MGLTextLanguage(textLanguage: "{old_name}", updatedTextField: "{new_name}")
    ],
    "place layer": [
        "{old_name}": MGLTextLanguage(textLanguage: "{old_name}", updatedTextField: "{new_name}")
    ],
    
]

The intermediate level of dictionaries is unnecessary, because we could represent exactly the same information like this:

[
    "water layer": MGLTextLanguage(textLanguage: "{old_name}", updatedTextField: "{new_name}"),
    "road layer": MGLTextLanguage(textLanguage: "{old_name}", updatedTextField: "{new_name}"),
    "place layer": MGLTextLanguage(textLanguage: "{old_name}", updatedTextField: "{new_name}"),
    
]

The way we apply localizedLayersByIdentifier is still problematic. If the developer:

  1. Sets MGLStyle.localizesLabels to YES
  2. Changes a particular style layer’s text from {name_fr} to {name_fr} ({ele})
  3. Sets MGLStyle.localizesLabels back to NO

then the style layer’s text gets overridden back to {name_en}. To avoid this problem, MGLTextLanguage should only keep track of which name_* property a given layer started out with, eliminating MGLTextLanguage:

[
    "water layer": "old_name",
    "road layer": "old_name",
    "place layer": "old_name",
    
]

/cc @fabian-guerra

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS localization Human language support and internationalization macOS Mapbox Maps SDK for macOS refactor labels Dec 16, 2017
@1ec5 1ec5 self-assigned this Dec 17, 2017
@1ec5
Copy link
Contributor Author

1ec5 commented Dec 18, 2017

The intermediate level of dictionaries appears to be used in order to store different values for different stops. This approach isn’t going to work with expressions (#8074), which can be much more complex.

Instead, one possible approach would be to:

  1. Wrap the overall expression in an assignment expression (let) that maps each unlocalized attribute to the localized attribute with predictably named variables.
  2. Replace every key path subexpression (get) referring to a name attribute with a variable expression (var) referencing one of the wrapper variables.

For example, if the system language is French and the localizesLabels option is set to YES, then:

TERNARY(
  FUNCTION(name, 'stringValue') = FUNCTION(name_en, 'stringValue'),
  name,
  FUNCTION(name, 'stringByAppendingString:', '\\n(', name_en, ')')
)

would become:

FUNCTION(
  TERNARY(
    FUNCTION($__com_mapbox_mapbox_streets__name, 'stringValue')
      = FUNCTION($__com_mapbox_mapbox_streets__name_en, 'stringValue'),
    $__com_mapbox_mapbox_streets__name,
    FUNCTION(
      $__com_mapbox_mapbox_streets__name,
      'stringByAppendingString:',
      '\\n(', $__com_mapbox_mapbox_streets__name_en, ')'
    )
  ),
  'mgl_expressionWithContext:',
  {
    $__com_mapbox_mapbox_streets__name = name_fr,
    $__com_mapbox_mapbox_streets__name_en = name_fr,
    $__com_mapbox_mapbox_streets__name_fr = name_fr,
    $__com_mapbox_mapbox_streets__name_es = name_fr,
    $__com_mapbox_mapbox_streets__name_zh_Hans = name_fr,
    …
  }
)

(Note that zh-Hans becomes zh_Hans due to the restricted character set for variable names: mapbox/mapbox-gl-js#5231.)

Then, if the localizesLabels option is turned back off, the context dictionary would become simply:

  {
    $__com_mapbox_mapbox_streets__name = name,
    $__com_mapbox_mapbox_streets__name_en = name_en,
    $__com_mapbox_mapbox_streets__name_fr = name_fr,
    $__com_mapbox_mapbox_streets__name_es = name_es,
    $__com_mapbox_mapbox_streets__name_zh_Hans = name_zh-Hans
    …
  }

None of the variable references would have to change, and the original evaluated values of the expressions – whatever attributes they referred to – would be restored.

Unfortunately, I think we should wait to implement this robust localization approach until we have a robust foundation to build it on. For one thing, it relies on being able to round-trip completely and reliably from an mbgl::style::expression::Expression to an NSExpression and back to an mbgl::style::expression::Expression. As a stopgap until #10714 is implemented, I’ve written a conversion from mbgl::style::expression::Expression to JSON-style objects to NSExpression, but the whole thing is incomplete and fragile, so I don’t think we should rely on it for code that runs nearly unconditionally on an existing style.

In the meantime, we can at least keep localizing constant values, relying on the fact that the text-field implementation in mbgl still recognizes the legacy {field} syntax, even inside literals in expressions.

/cc @anandthakker @tobrun

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 17, 2018

#10944 would have mbgl provide an expression transformation method. Since it would take an mbgl::style::expression::Expression as input, it would avoid some of the potential lossiness when roundtripping between Objective-C and C++ expression types.

@1ec5 1ec5 added this to the ios-v4.0.0 milestone Jan 18, 2018
@1ec5 1ec5 changed the title Automatic label localization is too complex Automatic label localization should comprehensively transform expressions Jan 24, 2018
@1ec5
Copy link
Contributor Author

1ec5 commented Jan 24, 2018

#10944 regresses localization of non-constant property values. Retitled this issue to track restoring localization of non-constant property values, which depends on a way to comprehensively transform an expression (#10944).

@jfirebaugh
Copy link
Contributor

I think we should consider approaches to label localization that don't rely on mutating style property values. That approach sounds complex to implement, requires coordination between the SDK implementers and the style authors, and turns out to be one of the causes of unexpectedly disabling style revalidation/refreshing (mapbox/mapbox-gl-js#4225 (comment)). (Furthermore, it's difficult to see how a "componentized" style would solve the refresh issue, because the nature of this strategy is to reach into whatever "component" has labels and mutate their internals.)

A better model would be to extend the expression language such that the style property expression could make reference to a bit of external state, controlled by the SDK, which indicates the desired localization. This external state would be decoupled from the style itself, which could remain independently refreshed. (I believe there have been several proposals to add this kind of parameterization mechanism.)

@1ec5
Copy link
Contributor Author

1ec5 commented Feb 20, 2018

A better model would be to extend the expression language such that the style property expression could make reference to a bit of external state, controlled by the SDK, which indicates the desired localization. This external state would be decoupled from the style itself, which could remain independently refreshed. (I believe there have been several proposals to add this kind of parameterization mechanism.)

I agree that localization would ideally be a built-in feature of the style language, rather than something grafted on at the platform-specific level. mapbox/mapbox-gl-js#6197 proposes a couple expression operators that would supplant the manual runtime styling operations that MGLStyle.localizesLabels currently performs.

The proposal isn’t quite as decoupled as you’re suggesting, because TileJSON doesn’t provide GL with enough context to decide which feature property to use for a given language. This isn’t a new problem, of course, but I feel better about hard-coding the Mapbox Streets source’s supported languages in an SDK feature than about doing the same as part of the style file format.

turns out to be one of the causes of unexpectedly disabling style revalidation/refreshing

There are many ways in which a developer would end up using the runtime styling API implicitly. Label localization via runtime styling isn’t specific to iOS/macOS; there’s a plugin for GL JS and soon there will be one for Android as well (mapbox/mapbox-plugins-android#74). All three localization tools are opt-in. The alternative is for application code to manually modify layers using runtime styling anyways.

Besides label localization, the iOS traffic plugin and the navigation SDK both use runtime styling to add overlays to the map. The documentation for these packages doesn’t explicitly mention runtime styling, either, but style revalidation is nonetheless disabled when using them.

While we wait for mapbox/mapbox-gl-js#6197, a short-term solution to the revalidation problem would be to document this behavior in the documentation for MGLStyle.localizesLabels, the traffic plugin, and the navigation SDK. Better documentation wouldn’t address the fundamental issue that these modifications happen behind the style author’s back, but it would head off some surprises.

For now, I think we should find a way to preserve the current level of support for label localization, kludgy as it may be, because the only alternative is currently to burden the developer with implementing their own MGLStyle.localizesLabels.

@1ec5
Copy link
Contributor Author

1ec5 commented Mar 21, 2018

Currently, localizesLabels can only replace constant values of the form {name_*} with constant values of the form {name_*}. This represents a regression from v3.7.x, in which camera and source functions could also be localized.

This implementation also relies on the legacy token syntax that has been removed from GL JS and will be removed from the native SDKs in #10399. Once that happens, localizesLabels will have no effect. (This is the same situation as in mapbox/mapbox-plugins-android#374.)

It would be straightforward to implement logic to modify a top-level key path expression. That would mostly get us back to the current state, but it not address the regression from v3.7.x, and concatenated values such as ["concat", "(", ["get", "name_*"], ")"] would still regress. Also, if the style comes with its own name fallback logic via conditional or coalescing expressions, localizesLabels won’t work.

The Cartography team is planning to introduce additional non-name fields that might be used as fallbacks for when no name is available in the user’s language, but these fields may not match one-for-one with name fields, which is the same problem we have with the English-only abbr field (#9902). So the requirements for iOS SDK v4.0.0 given these complexities seem to be:

  • Modify key path expressions within step and interpolating expressions
  • Modify key path expressions within conditional and coalescing expressions
  • Parse out concatenating and other expressions
  • Localize fields other than name_*

/cc @langsmith @nickidlugash

@jfirebaugh
Copy link
Contributor

This implementation also relies on the legacy token syntax that has been removed from GL JS and will be removed from the native SDKs in #10399.

Correction on this point: support for the token syntax has not and is not being removed; existing styles that use tokens will continue to work. #10399 is about avoiding a Hyrum's Law situation by not shipping a release in which it's possible to mix the expressions and token syntaxes.

@1ec5
Copy link
Contributor Author

1ec5 commented Mar 28, 2018

Thanks for the clarification. Regardless, core styles already use style functions and will be transitioning to interpolation expressions, so we’ll need to incorporate a small degree of non-constant expression support in localizesLabels or these styles will begin to appear unlocalized to the user.

@1ec5 1ec5 changed the title Automatic label localization should comprehensively transform expressions Automatic label localization should transform non-constant expressions Mar 30, 2018
@1ec5
Copy link
Contributor Author

1ec5 commented Apr 6, 2018

The iOS navigation SDK has its own implementation of automatic label localization, since that library needs to use the local language to match the Direction API’s unlocalized response. mapbox/mapbox-navigation-ios#1076 updates that implementation for expressions. Tokens continue to be replaced, but now it digs into interpolation and step functions instead of style functions. The only new kind of expression that it localizes is key path expressions. I’m planning to upstream the navigation SDK implementation into this SDK, porting it to Objective-C.

@1ec5
Copy link
Contributor Author

1ec5 commented Apr 16, 2018

Fixed in #11651 on the release-boba branch for iOS map SDK v4.0.0 and macOS map SDK v0.7.0.

@1ec5 1ec5 closed this as completed Apr 16, 2018
@1ec5
Copy link
Contributor Author

1ec5 commented Apr 16, 2018

With respect to the “delocalization” issues in #10713 (comment), note that #11651 essentially removes the ability to automatically delocalize a style: #11651 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS localization Human language support and internationalization macOS Mapbox Maps SDK for macOS refactor
Projects
None yet
Development

No branches or pull requests

2 participants