Closed Bug 1196949 Opened 9 years ago Closed 8 years ago

Use Cancel string provided by Firefox language file

Categories

(Firefox for iOS :: Browser, defect)

All
iOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
fxios-v3.0 --- affected
fxios-v4.0 --- fixed
fxios 4.0+ ---

People

(Reporter: Tonnes, Assigned: st3fan)

References

()

Details

Attachments

(3 files, 3 obsolete files)

When testing by Onno and looking at the screenshot (see URL), it turns out the Cancel string in one particular string/screen is probably obtained from iOS itself, resulting in conflicting wording style.

Instead, it should use all Firefox strings by default like the other Cancel occurrences in Firefox. There may also just be a need for an additional entry for this screen only, similar to the Add string.
Assignee: nobody → sleroux
Attachment #8655444 - Flags: review?(sarentz)
Attachment #8655446 - Flags: review?(sarentz)
I also updated the system buttons that used the Done action instead of our localized copy.
Merged.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
(In reply to Stephan Leroux [:sleroux] from comment #3)
> I also updated the system buttons that used the Done action instead of our
> localized copy.

Either l10n comment is misleading (all the title occurrences instead of button label) or this effectively reuses one string for button labels and other elements which will not work well.

https://github.com/mozilla/firefox-ios/commit/6eb78d27624fbcd116a03cc9ca5a25650fd11246
Flags: needinfo?(sleroux)
Sorry about that - ya 'button label' is probably better than 'title'. I can submit a patch that will fix that. Thanks!
Flags: needinfo?(sleroux)
I've updated the comment for the done buttons. Does this work?
Attachment #8655976 - Flags: feedback?(splewako)
Comment on attachment 8655976 [details] [review]
https://github.com/mozilla/firefox-ios/pull/1017

Yes, this makes comments for "Done" more understandable, thx.

In the same time I wonder why "Cancel title from search engine picker" and "Cancel title for selecting client screen" comments from 6eb78d27624fbcd116a03cc9ca5a25650fd11246 commit do not show up in exported xliff.
Attachment #8655976 - Flags: feedback?(splewako) → feedback+
Looks like there was no tableName specified for the strings I added. Also looks like this might be the cause for a bunch of other strings. I've filed a follow bug for to investigate these warnings here: https://bugzilla.mozilla.org/show_bug.cgi?id=1201102
This should be reopened as the patch doesn't seem to work as expected, https://bugzilla.mozilla.org/attachment.cgi?id=8663926
Flags: needinfo?(sleroux)
I wonder if the export conflict is causing this string to not be shown

https://bugzilla.mozilla.org/show_bug.cgi?id=1201102

Will reopen and investigate
Status: RESOLVED → REOPENED
Flags: needinfo?(sleroux)
Resolution: FIXED → ---
I doubt it has anything to do with bug 1201102

I think the problem here is that the Cancel string is missing the tableName: "SendTo". Without that, the string ends up in the main strings table and I'm pretty sure the extension cannot access strings from the main bundle.
Ah that makes more sense. How would that impact our string freeze? Simple fix code wise.
Flags: needinfo?(sarentz)
OS: iOS 8 → iOS
Hardware: Other → All
Blocks: iosl10n
We cannot fix this because we are past string freeze.

What I would like to suggest is that we revert the change. Instead of the NLocalizedString() strings we go back to using UIBarButtonSystemItem.Cancel and UIBarButtonSystemItem.Done

We will fix this as soon as possible in a followup release.

I understand that this means that we will use language that :stef prefers not to use, but it will be 'iOS standard' so I think it is a good short term compromise.
Flags: needinfo?(splewako)
Flags: needinfo?(sarentz)
Flags: needinfo?(francesco.lodolo)
(In reply to Stefan Arentz [:st3fan] from comment #17)
> I understand that this means that we will use language that :stef prefers
> not to use, but it will be 'iOS standard' so I think it is a good short term
> compromise.

Sounds good for me, and I think the issue was more for Ton (and Dutch) who reported the bug originally.
Flags: needinfo?(francesco.lodolo)
I wouldn't care about string freeze but exactly what Flod said, in Polish there is no difference - unless the sting is in English of course…
Flags: needinfo?(splewako)
(In reply to Francesco Lodolo [:flod] from comment #18)
> Sounds good for me, and I think the issue was more for Ton (and Dutch) who
> reported the bug originally.

As well as other locales suffering this. Just to be sure: it couldn’t be (temporarily) fixed just by adding a Cancel string for that screen only, as mentioned in comment 0?
Blocks: 1212574
As a compromise we have just landed bug 1214676

This will use a system provided Cancel button (UIBarButtonSystemItem.Cancel) in the Send To and Share To dialogs.

When the string windows opens again for the next release (either bugfix or big release) we can work on this bug again.

(This bug will then be about reverting bug 1214676 and making sure the strings have the correct table name specified)
Whiteboard: [needstrings]
Attaching screenshot to the bug just in case.
Attachment #8655444 - Attachment is obsolete: true
Attachment #8655446 - Attachment is obsolete: true
Attachment #8655976 - Attachment is obsolete: true
Attachment #8736002 - Flags: review?(sarentz)
Attachment #8736002 - Flags: review?(sarentz) → review?(bnicholson)
Attachment #8736002 - Flags: review?(bnicholson) → review+
master f998f960111e14c8c88f622c1c0056a558d24180
Status: REOPENED → RESOLVED
Closed: 9 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.0
Whiteboard: [needstrings] → [needsuplift]
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8747743 - Flags: review?(sleroux)
Assignee: sleroux → sarentz
Attachment #8747743 - Flags: review?(sleroux) → review+
Comment on attachment 8747743 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/1768

If you say it works, I'll take your word for it!

I don't like the fact that this requires another argument to NSLocalizedString -- that's just one more thing for us to get wrong. I'll keep playing around with strings to see if we can simplify this process more. I'm still not happy with it :(
Attachment #8747743 - Flags: review?(bnicholson) → review+
Whiteboard: [needsuplift]
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: