Closed Bug 1207108 Opened 9 years ago Closed 9 years ago

Duplicated 'bookmarks_title' string in android_strings.dtd

Categories

(Firefox for Android Graveyard :: Data Providers, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox43 fixed, firefox44 fixed, fennec43+)

RESOLVED FIXED
Firefox 44
Tracking Status
firefox43 --- fixed
firefox44 --- fixed
fennec 43+ ---

People

(Reporter: flod, Assigned: ally)

References

Details

Attachments

(1 file)

We don't have any test or similar to find duplicate keys, this was randomly found by another tool that thought this was a string changed without a new ID.

http://hg.mozilla.org/mozilla-central/diff/26b367ff0033/mobile/android/base/locales/en-US/android_strings.dtd
This moved 'bookmarks_title' from bookmarks.inc to android_strings.dtd, value of the string "Mobile".

The problem is that we already have a string with this id, value "Bookmarks"
http://hg.mozilla.org/mozilla-central/file/3d97a673734c/mobile/android/base/locales/en-US/android_strings.dtd#l27

I tried to check the code but I couldn't figure out if we're actually still using the "Mobile" string. The "Bookmarks" one should be the one used for the home panel.

I think we should try to fix this as soon as possible, and probably break string freeze in aurora. No idea what localization tools will do, if they'll expose the first string or the second for localization.
Looks like Pootle is ignoring the first occurrence of the string, which is pretty bad considering it's the most important of the two

The string has been "untranslated" and the product results complete in Pootle's dashboard
http://hg.mozilla.org/releases/l10n/mozilla-aurora/cy/diff/21bfcad6f83b/mobile/android/base/android_strings.dtd
Flod, do you know which versions this is a problem in? Is it only 43 & 44?
tracking-fennec: --- → ?
Flags: needinfo?(francesco.lodolo)
We don't obviously to actually consume the bookmarks_title value which from the bookmarks.inc file. 

The area of concern on IRC is whether this is used to create the mobile bookmark root. Looks like we use the guid on creation, or when manipulating mobile bookmarks

code looks like 
       final long folderID = getFolderIdFromGuid(cr, Bookmarks.MOBILE_FOLDER_GUID);
        if (folderID == FOLDER_NOT_FOUND) {
            Log.e(LOGTAG, "No mobile folder: cannot add distribution bookmarks.");
            return offset;
        }
which is defined as
public static final String MOBILE_FOLDER_GUID = "mobile"; in BrowserContract.java

so I think we're in the clear to just remove the from the old .inc from the dtd file.
tracking-fennec: ? → ---
Can't be any other versions, it came in from http://hg.mozilla.org/mozilla-central/rev/26b367ff0033, Bug 1197054 - Build changes to get rid of bookmarks.inc
Assignee: nobody → ally
Attachment #8664385 - Flags: review?(nalexander)
Sadly it's in 42, which means we need to break string freeze in Aurora to fix it and let localization tools pick up the right string.
Flags: needinfo?(francesco.lodolo)
Comment on attachment 8664385 [details] [diff] [review]
idCollisionForBookmarkStrings

Review of attachment 8664385 [details] [diff] [review]:
-----------------------------------------------------------------

I trust you got the right one.
Attachment #8664385 - Flags: review?(nalexander) → review+
(In reply to Francesco Lodolo [:flod] from comment #6)
> Sadly it's in 42, which means we need to break string freeze in Aurora to
> fix it and let localization tools pick up the right string.

Brain failure. I meant 43 and 44, the former being now in Aurora, not 42.
tracking-fennec: --- → 43+
https://hg.mozilla.org/integration/fx-team/rev/4ac91cde9a3d299dc3c05d1c6d247ba30d0ff3be
Bug 1207108 - Duplicated 'bookmarks_title' string in android_strings.dtd.r=nalexander
https://hg.mozilla.org/mozilla-central/rev/4ac91cde9a3d
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Comment on attachment 8664385 [details] [diff] [review]
idCollisionForBookmarkStrings

Approval Request Comment
[Feature/regressing bug #]: duplicated string ID breaks localization tools that expose only one of the two English strings.
[User impact if declined]: localized builds might have wrong translation for the home panel Bookmarks.
[Describe test coverage new/current, TreeHerder]: -
[Risks and why]: low.
[String/UUID change made/needed]: removed one unused string.
Attachment #8664385 - Flags: approval-mozilla-aurora?
Comment on attachment 8664385 [details] [diff] [review]
idCollisionForBookmarkStrings

Fix a basic issue, taking it.
Attachment #8664385 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: