New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 719409 link

Starred by 3 users

Issue metadata

Status: Archived
Owner:
Closed: Jan 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

There seem to be three copies of some strings in .grd files

Project Member Reported by benwells@chromium.org, May 8 2017

Issue description

E.g. IDS_INFOBAR_MISSING_LOCATION_PERMISSION_TEXT is in three files. Two of these are identical ("Chrome needs") and the other is the same but with chromium instead of chrome.

See https://cs.chromium.org/search/?q=IDS_INFOBAR_MISSING_LOCATION_PERMISSION_TEXT&type=cs for all three.

I can think of two improvements:
1. Remove one of the two identical strings. If there is a need sometimes to have different strings for different products, have one of the files take priority and only put in duplicate definitions when the strings are different
2. Remove the chromium specific one and have a placeholder for the product name.

Maybe both of these things already exist (I thought 2 did) but this string and others like it are just being used improperly?

Not sure who looks after this stuff so adding two TPMs who might know...
 
Labels: OS-All
dimu@, can you PTAL?
dimu@, can you PTAL?
Owner: dimu@chromium.org
Status: Assigned (was: Untriaged)
Ugh, thanks a lot Monorail outage.  Sorry for comment spam.

dimu@, can you PTAL?

Comment 4 by dimu@google.com, May 10 2017

Cc: dimu@chromium.org
Owner: tedc...@chromium.org
I not very familiar about how the .grd are being used by different products.

Re-assign to tedchoc@ who added these IDs. Would you please take a look? Thanks!
Cc: tedc...@chromium.org aga...@chromium.org
Owner: ----
Status: Available (was: Assigned)
If I recall (and I know very little of the capabilities of grd, so I'm unassigning this from myself as I'm not going to work towards any resolution), we use this string in both java and C++ code.

Again to the best of my recollection, formatter_data="android_java" is the normal way of sharing strings across java and C++, but at the time chromium_strings.grd and google_chrome_strings.grd did not support it and would not be able to generate the java one we needed.

I would love to know if the product name replacement works, but I haven't seen it anywhere either.  You need to find someone familiar with strings at this point, and I don't know who that is (maybe agable@?)

Comment 6 by aga...@chromium.org, May 10 2017

Cc: -aga...@chromium.org
Nope, not me :( I only know things about the process of copying strings into prod, and copying translations out. I know nothing else.
Cc: osh...@chromium.org
+oshima who i've heard knows about grd files
Cc: tapted@chromium.org
And +tapted who knows about everything
Cc: thakis@chromium.org
+thakis since he's in tools/grit/OWNERS.  I suspect many of these questions are more about the tools than the grd format.  I fear in two more steps we will have all chromium engineers on this bug though :-P
chrome/app/chromium_strings.grd and chrome/app/google_chrome_strings.grd are supposed to have identical strings except for product name. Using placeholders tends to not work well in non-english languages due to inflections and what have you.

I don't know what chrome/android/java/strings/android_chrome_strings.grd is for. If comment 5 is still true, we should probably find a way to make chrome/app/{chromium,google_chrome}_strings.grd usable from java.
Ted - is there any point in having chromium versions of the android only strings? Currently it seems like the java version will be chrome not chromium, so if it is useful we'll get a weird mix.
@#11, I don't know the historical reasoning between the split on desktop.  I certainly have no intention of adding that complexity to the Android strings if we can avoid it, which we have for the last 6ish years.

I'm actually surprised placeholders aren't ok for the product name as I thought we intentionally do not translate it.  Granted, I don't know if there are occurrence of longer Product-y names like "Google Chrome" where you could imagine some language that would convert to "Chrome by Google".
Owner: benwells@chromium.org
Status: Assigned (was: Available)
I just had a bit of a lookie and see a few options to improve things:

1. set up the formatter_data=android_java stuff for the branded strings (chrome/app/{chromium,google_chrome}_strings.grid).

This doesn't look too hard but it might get fiddly. If we do this there will just be two versions of these strings - one for Chrome and one for Chromium. This might not be needed for Android strings where we don't ship a chromium, but it means that branded strings where the same string is used on desktop and also in java code for Android would be supported.

2. just move the branded Android only strings that are needed in java into generated_resources.grd.

This would be super simple to do. It would not support branded strings used in java and on destkop, and would introduce branded strings into generated_resources which is maybe bad (there seem to be quite a few there already but that maybe should be fixed).

3. Make the branded strings be .grdp files conditionally included into generated_resources.grd. I think this would have the same effect as 1. but would probably be a lot simpler.

I'm going to have an attempt at 3 as it seems like the proper way to do things and will make things simpler. If it is a stupid idea for some reason please let me know!
Cc: -amineer@chromium.org
Feel free to re-CC me but I'm not sure any of the active discussion needs my blessing.  I do appreciate you reviewing this though, thanks!
Another duplication just appeared (in https://codereview.chromium.org/2912133002):

chrome/app/generated_resources.grd: IDS_INTENT_PICKER_BUBBLE_VIEW_INCOGNITO_*
chrome/android/java/strings/android_chrome_strings.grd: IDS_EXTERNAL_APP_LEAVE_INCOGNITO_*

No apparent way around it right now but if a solution comes up, those should be part of it.
Labels: Performance-Size
Some thoughts on how to access .pak strings from .java in  bug 710081 
Cc: digit@chromium.org
+digit

From an email by digit@:

I'm currently working on localized strings support for Chrome on Android. Yes, the topic can be really confusing :)

Looking into your issue, it looks like the string you're trying to use in generated_resources.grd is inside an <if expr="chromeos"> element, as such it is never placed into the generated R.java when grit is invoked with the "-t android" option. In other words, while these strings appear in two different files, they are not duplicated in the final binaries.

I've just tried this locally and it worked. On the other hand, I don't see any use of IDS_INTENT_PICKER_BUBBLE_VIEW_INCOGNITO_WARNING et al in the source tree, and I wonder if these definitions are obsolete now? If so you might want to remove them from generated_resources.grd instead? If not, the ID name doesn't make much sense for Android and might probably be changed too (e.g. IDS_NAVIGATION_INCOGNITO_WARNING).
…

Is there an existing mechanism for accessing strings defined in generated_resources.grd from Java?

The mechanism seems to be:
add formatter_data="android_java" to every <message> element. Better place these elements inside an <if expr="is_android"> element if they are specific to Android (and of course ensure they are not filtered out, like in your case).

If you add a new .grd file, ensure that there is a java_strings_grd("my_new_grd_strings") rule in your BUILD.gn to properly process it with grit and generate the corresponding .xml files.

Add the corresponding target (e.g. my_new_grd_strings) is listed in the deps section of the android_resources("chrome_java_resources") definition in chrome/app/BUILD.gn to ensure that said resources will be part of the final org/chromium/chrome/R.java file.
For the record, the R.java source file is located inside of $OUT/gen/chrome/android/chrome_java_resources.srcjar, in case you need to take a look at it to check everything :)
digit: thanks for all the info, it sounds like you know what you're talking about :)

The case I've been looking at is for the branded resource files, which don't have the android_java stuff set up yet. Could you take a look at #13 and let me know what you think?

I had another look today at this as I should have time this week to spend on it. I think taking option 1) will result in lots of stuff being added to the BUILD file for the various languages that need to be supported.
Ignore #15, it turns out there's a simple solution spotted by digit@ (I was trying to use an <if expr="chromeos"> string from Android).
digit - ping re #18. Could you provide some advice? After looking into it a bit I've decided this really needs whoever the expert(s) in resources is / are to fix.

Comment 21 by digit@chromium.org, Jul 19 2017

I just took a quick look at the .grd files:

- chrome/android/java/strings/android_chrome_strings.grd contains strings
  that are exclusively used by Java code from chrome/android/java/.

  There are many messages defined here that contain Chrome or Google, but
  the vast majority of them do not.

- chrome/android/webapk/strings/android_webapk_strings.grd is exactly in
  the same situation, btw.

I agree that option 1. seems the best, i.e.: get rid of these two files by moving the non-branded messages to generated_resources.grd, and the branded ones to ${BRAND}_strings.grd files (if they don't already exist there).

For the latter, this means duplicating the messages by replacing Chrome with Chromium, even though we don't plan to release Chromium builds for Android ourselves (but someone else may want to do just that, and shouldn't have to suffer from our historical inconsistencies).

This should not require any line of Java source files, since all strings are exported by the same auto-generated org.chromium.chrome.R class in the end.

Of course, this assumes that adding formatter_data="android_java" as appropriate works as expected. I'm really curious why this would not be the case.

Keep in mind that you will also have to move the java_strings_grd() calls from chrome/android/BUILD.gn to chrome/app/BUILD.gn in order to ensure that the Android .xml string resource files are generated properly for the branded messages.

Option 2. would be definitely bad and confusing for everyone :-/

Option 3. might work and actually simplify the BUILD.gn files, but I'm not sure how to properly set the proper conditions for GRIT to include either the google_chrome or chromium .grdp files.

Let me know if this isn't very clear :)

Comment 22 by agrieve@chromium.org, Jan 21 (2 days ago)

Status: Archived (was: Assigned)

Sign in to add a comment