Closed Bug 1207108 Opened 8 years ago Closed 8 years ago

Duplicated 'bookmarks_title' string in android_strings.dtd


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

Not set


(firefox43 fixed, firefox44 fixed, fennec43+)

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


(Reporter: flod, Assigned: ally)




(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.
This moved 'bookmarks_title' from to android_strings.dtd, value of the string "Mobile".

The problem is that we already have a string with this id, value "Bookmarks"

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
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 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

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, Bug 1197054 - Build changes to get rid of
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]

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+
Bug 1207108 - Duplicated 'bookmarks_title' string in android_strings.dtd.r=nalexander
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Comment on attachment 8664385 [details] [diff] [review]

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]

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.