"Enable location data" string displays untranslated

Bug #1393438 reported by Sebastien Bacher
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical System Image
Fix Released
Undecided
Unassigned
location-service (Ubuntu)
Invalid
Undecided
Unassigned
unity-scopes-api (Ubuntu)
Invalid
High
Michi Henning
unity8 (Ubuntu)
Fix Released
High
Albert Astals Cid

Bug Description

Using rtm 162 on krillin, several of the dash screens have a "enable location data" option, that strings seems to not be translatable

Related branches

Revision history for this message
Sebastien Bacher (seb128) wrote :

The string comes from unity-scopes-api which is not using gettext (that project seems to also not be setup for translations)

tags: added: rtm14
Changed in unity-scopes-api (Ubuntu):
importance: Undecided → High
tags: added: touch touch-l10n
Revision history for this message
Michi Henning (michihenning) wrote :

That string comes from the <scope-id>-settings.ini file, which is provided by the scope. The display name for settings can be localised: http://developer.ubuntu.com/scopes/tutorials/adding-settings-to-your-scope/

Revision history for this message
Chris Wayne (cwayne) wrote :

@Michi actually that particular string is done from unity-scopes-api, and is automatically placed there when LocationDataNeeded is set to true. Many of my scopes don't even have a -settings.ini file and still have that setting

Revision history for this message
Michi Henning (michihenning) wrote :

Right you are! Sorry, I forgot about the special status of the LocationDataNeeded, which is automagically added by the shell.

This is actually not a scopes-api issue, it's the shell that's not translating this.

Changed in unity-scopes-api (Ubuntu):
status: New → Invalid
Revision history for this message
Albert Astals Cid (aacid) wrote :

Why is the shell responsible of any of this? It's a string that is not showing at all in our code, so we should not be responsible of translating it.

As Sebastian says it comes from unity-scopes-api so it is unity-scopes-api that should make it translatable.

./src/scopes/internal/IniSettingsSchema.cpp:297: Setting s("internal.location", "boolean", "Enable location data", VariantArray(), Variant(true));
./src/scopes/internal/JsonSettingsSchema.cpp:418: Setting s("internal.location", "boolean", "Enable location data", VariantArray(), Variant(true));

Changed in unity8 (Ubuntu):
status: New → Invalid
Changed in unity-scopes-api (Ubuntu):
status: Invalid → New
Revision history for this message
Michał Sawicz (saviq) wrote :

Totally, we don't know what strings you'll be sending our way, and trying to maintain a list is bound to fail sooner rather than later. You need to allow translating along with the project where the strings are in code.

Revision history for this message
Pete Woods (pete-woods) wrote :

Michi, the shell doesn't add this - it's done inside the scopes runtime. If the shell was responsible, then another client wouldn't see the location setting. It seems to be like unity-scopes-api needs to add localisation support in order to be able to translate any user-visible strings. I guess this is the first one, though?

Revision history for this message
Michał Sawicz (saviq) wrote : Re: [Bug 1393438] Re: "Enable location data" string displays untranslated

W dniu 19.11.2014 o 10:47, Pete Woods pisze:
> Michi, the shell doesn't add this - it's done inside the scopes runtime.
> If the shell was responsible, then another client wouldn't see the
> location setting. It seems to be like unity-scopes-api needs to add
> localisation support in order to be able to translate any user-visible
> strings. I guess this is the first one, though?

I totally see the problem with enabling translations for a single string
(though I imagine there could be more in the future?).

Maybe we need to come up with a better approach? I.e. you telling the
shell "this is a setting item for location" and we'll handle the whole
thing on our side then?

Revision history for this message
Michi Henning (michihenning) wrote :
Download full text (3.2 KiB)

Hmmm... I can (mostly) recall the discussion we had with Michal around that time (months ago). Back then, we decided that all the scope does is set a LocationDataNeeded item to true in the scope.ini file. That setting becomes available to the shell via the scope's metadata. The shell is now responsible for adding a way for the user to allow/fuzz/disallow the location data in the scope's settings menu.

We decided to do this because

- we didn't think it was reasonable for a scope to have to create a settings.ini file just so the scope can get location data. (That's messy because some scopes will have location data as their *only* setting.)

- If each scope is responsible for creating its own settings.ini entry for location data, what is essentially the same setting for all scopes needs to be translated separately for each scope. That's a sure-fire way to end up with inconsistent strings for this setting for different scopes. It's a tri-state setting, with values "on, off, approximate". (At least, that's what we were expecting at the time.) Besides, each and every scope would have to make sure that it uses the exact same definition for the name, type, and values of the enum.

- If the meaning of the location data setting ever changes in the future (e.g., we change it to "off, precise to 100 km, precise to 10 km, full precision"), the setting metadata definition is baked into each and ever scopes settings.ini file, where we can't change it. For this reason, we also decided that the user-select value of the setting should *not* be stored in the scope's normal settings database, but somewhere else.

- Even if each scope were to create it's own settings.ini file, the shell would *still* have to interpose itself in some way. Otherwise, the scope could ship a settings.ini file that adds location data, sets the default value to "true" (or "precise"), and the scope would get location data without the user ever having given permission.

I just had a look through the code. The string was added with commit 448. Pete, I suspect that you were unaware of what we had decided earlier when you made that change and, my apologies, I somehow missed this during review. The intent all along was for the shell to notice the LocationDataNeeded value in the metadata and to construct the additional settings menu item in response to that.

So, just fixing the translation in the scopes-api won't really fix the problem. Conceptually, the location data needed thing is much like the rendering preferences that a scope indicates to the shell: the scope just says "I want this", and the shell takes care of cooking this up in the UI and making the state persistent. We have to make sure that:

- If we ever change the location data meaning, we can do so in the future without breaking every installed scope.

- That a scope can't trick the machinery into providing location data without explicit user permission.

- That the user-selected value is not stored in the normal scope settings DB.

I'm reluctant to add translations to the scopes api, not because it's more work, but because I think the string shouldn't be there in the first place (and it's the only one). If we end up with UI ...

Read more...

Revision history for this message
Michał Sawicz (saviq) wrote :

Right, what you wrote above I totally agree with, but it's not how it's currently implemented, and we were not aware this is how it was supposed to work. We just got a new boolean setting and we dealt with it as with any other.

Now, to rectify this untranslatable string somewhat quickly, I propose the setting comes of "location" instead of "boolean" type, which we'd then render as we please, including the currently-untranslatable string.

Long-term, IMO we should make use of the trusted location helper, which would take care of ensuring the scope in question is allowed to get location data (and of what accuracy), the dash would either act as a proxy, as it does today, or the scope will request data from the location service directly, but we'd then need a way to link the scope process to a surface on which to open the location trust store dialog. That would be useful for other use cases as well (payments, accounts etc.), so might be a nobler goal.

Changed in unity-scopes-api (Ubuntu):
status: New → Confirmed
status: Confirmed → Triaged
Changed in unity8 (Ubuntu):
status: Invalid → Confirmed
Changed in unity-scopes-api (Ubuntu):
assignee: nobody → Michi Henning (michihenning)
Revision history for this message
Michi Henning (michihenning) wrote :

OK, the linked branch should fix this. To make the transition easier, I have *added* a new settings definition called internal.location.precision. It's an enum with values "None", "Approximate", and "Precise". The old internal.location boolean setting is still there. The idea is to make the transition easier: we can check this in now without affecting the shell, and when the shell is ready to change its end of the fix, it can start using internal.location.precision instead of internal.location.

Note that not only the "Enable location data" string will need localizing, but also the three enumerator values.

Changed in unity-scopes-api (Ubuntu):
status: Triaged → In Progress
Revision history for this message
Michi Henning (michihenning) wrote :

One more note: while fixing this, I found a bug in our code: for settings of enum type, if the settings schema was created from a .ini file, the VariantMap returned by the registry metdata() method returned the enumerators in a field named "displayValues". However, when the settings schema was created from a JSON string (as is the case for remote scopes), the same thing in in the VariantMap was in a field named "values".

I'm not sure whether that affects the shell; probably best to double-check. It is "displayValues" for both now. If the shell looks for "values" as the key for enum definitions in the scope metadata, it needs to use "displayValues" instead.

Revision history for this message
Michi Henning (michihenning) wrote :

See the linked branch for more discussion. We are back to the original internal.location setting now.

Changed in unity-scopes-api (Ubuntu):
status: In Progress → Invalid
Revision history for this message
Sebastien Bacher (seb128) wrote :

That's still an issue on r228 and quite visible since that string is in the configuration option of most scopes, could we get assigned/triaged at least?

Michał Sawicz (saviq)
Changed in unity8 (Ubuntu):
status: Confirmed → Triaged
importance: Undecided → High
assignee: nobody → Andrea Cimitan (cimi)
Changed in unity8 (Ubuntu):
assignee: Andrea Cimitan (cimi) → Albert Astals Cid (aacid)
status: Triaged → In Progress
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package unity8 - 8.02+15.04.20150216.1-0ubuntu1

---------------
unity8 (8.02+15.04.20150216.1-0ubuntu1) vivid; urgency=medium

  [ Andrea Cimitan ]
  * Fix temp scopes opening temp scopes, correctly close previously
    opened temp scope with its preview (LP: #1410337)
  * Set sourcesize for scopes images in manage dash

  [ CI Train Bot ]
  * Resync trunk

  [ Albert Astals Cid ]
  * Test for bug #1316660 (LP: #1316660)
  * Hardcode tranlsation for internal.location field (LP: #1393438)
  * Make sure the height it's the height it will have

  [ Michael Zanetti ]
  * Cleanup cmake warning about missing Qt5Sql module
 -- CI Train Bot <email address hidden> Mon, 16 Feb 2015 13:14:26 +0000

Changed in unity8 (Ubuntu):
status: In Progress → Fix Released
Changed in location-service (Ubuntu):
status: New → Invalid
Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

Closing C.S.I task because it is fixed in stable builds.

Changed in canonical-devices-system-image:
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.