Duplicated 'bookmarks_title' string in android_strings.dtd

RESOLVED FIXED in Firefox 43

Status

()

Firefox for Android
Data Providers
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: flod, Assigned: ally)

Tracking

Trunk
Firefox 44
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox43 fixed, firefox44 fixed, fennec43+)

Details

Attachments

(1 attachment)

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)
(Assignee)

Comment 3

2 years ago
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: ? → ---
(Assignee)

Comment 4

2 years ago
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)

Comment 5

2 years ago
Created attachment 8664385 [details] [diff] [review]
idCollisionForBookmarkStrings
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.

Updated

2 years ago
tracking-fennec: --- → 43+
(Assignee)

Comment 9

2 years ago
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
Last Resolved: 2 years ago
status-firefox44: affected → fixed
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+
https://hg.mozilla.org/releases/mozilla-aurora/rev/ac18fcc6bb9f
status-firefox43: --- → fixed
You need to log in before you can comment on or make changes to this bug.