760804 - PDF viewer should use a single unicode character instead of "..." and should have localization comments

Status: RESOLVED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Francesco Lodolo [:flod]]

http://hg.mozilla.org/mozilla-central/file/65f748c63f4c/browser/locales/en-US/pdfviewer/viewer.properties

loading=Loading... {{percent}}%

All Mozilla's products use "…" instead of "...". Also, that file need at least an initial localization comment explaining what should be left untranslated (variables).

text_annotation_type=[{{type}} Annotation]
This string needs a l10n comment explaining what "type" could be.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 2

[Axel Hecht [:Pike]]

Comment on attachment 629965 [details] [diff] [review]
patch

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

::: browser/locales/en-US/pdfviewer/viewer.properties
@@ +11,3 @@
>  page_label=Page:
>  page_of=of {{pageCount}}
> +

I don't expect this to work in general. I'd expect a combi of double plurals here, similar to what we do in download manager. Notably, there could be text that depends on the current page number preceding the current page number.

I know this is a comment fix, but it makes the code problem more obvious.

@@ +24,5 @@
>  download_label=Download
>  bookmark.title=Current view (copy or open in new window)
>  bookmark_label=Current View
>  
> +# Side panel toolbar button tooltips

As a non-native speaker, I can't parse this string.

@@ +37,1 @@
>  thumbs_label=Thumbnails

What's the point of the re-ordering here? Why's thumbs_label separate from thumbs.title?

(yeah, _ and ., sob).

@@ +49,1 @@
>  thumb_page_canvas=Thumbnail of Page {{page}}

I wonder if these should use plural forms. flod?

@@ +83,5 @@
>  loading_error_indicator=Error
>  loading_error=An error occurred while loading the PDF.
>  
>  # Misc labels and messages
> +# LOCALIZATION NOTE (text_annotation_type): "{{[percent}}" will be replaced with a percentage

Can't match this comment to the string.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to Axel Hecht [:Pike] from comment #2)
> I know this is a comment fix, but it makes the code problem more obvious.

Yes, indeed. Too late to change this now, though... We can follow up on trunk?

> > +# Side panel toolbar button tooltips
> 
> As a non-native speaker, I can't parse this string.

Would "Tooltips for side panel toolbar buttons" be clearer?

> What's the point of the re-ordering here? Why's thumbs_label separate from
> thumbs.title?

I thought it'd make it clearer how the strings differed. Really these should use .label/.tooltip, but changing the string names is too much of a PITA at this point.

> > +# LOCALIZATION NOTE (text_annotation_type): "{{[percent}}" will be replaced with a percentage
> 
> Can't match this comment to the string.

Fixed this per discussion on IRC:
-# LOCALIZATION NOTE (text_annotation_type): "{{[percent}}" will be replaced with a percentage
+# LOCALIZATION NOTE (text_annotation_type): This is used as a tooltip.
+# "{{[type}}" will be replaced with an annotation type from a list defined in
+# the PDF spec (32000-1:2008 Table 169 – Annotation types).
+# Some common types are e.g.: "Check", "Text", "Comment", "Note"

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Attachment: updated patch

Comment 5

[Yury Delendik (:yury)]

> I thought it'd make it clearer how the strings differed. Really these should use .label/.tooltip, but changing the string names is too much of a PITA at this point

Currently PDF.js is using webL10n for localization of the HTML code. The dot sign is reserved for separation the element localization id from the HTML attribute name. So it will not be possible to change "xxx.yyy" to "xxx_yyy" and vice verse, without adding the complexity to the HTML code localization.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: updated again

Note that the only functional change here is the change to the ellipsis.

Comment 7

[Axel Hecht [:Pike]]

Comment on attachment 629987 [details] [diff] [review]
updated again

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

The tooltip vs title story seems to be awkward, but not part of this patch, f=me to get some comments in for our l10n community.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eee5cd8316af
https://hg.mozilla.org/releases/mozilla-aurora/rev/36066dd58839

Comment 9

[Francesco Lodolo [:flod]]

> @@ +49,1 @@
> >  thumb_page_canvas=Thumbnail of Page {{page}}
> 
> I wonder if these should use plural forms. flod?

I don't think so, I expect {{page}} to be a page number, and thumbnail to remain singular.

Unfortunately all these features land way too late on trunk, so we'll never get the chance to fix them before the switch to aurora (another example is GLCI, I just found 2 hard-coded strings...).

Another doubt:
page_scale_width=Page Width
page_scale_fit=Page Fit

I expect the first to be "Fit to Width" and the second "Fit to Page", at least if I got the real meaning of the first right.

Comment 10

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/mozilla-central/rev/eee5cd8316af