Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Utilize Fluent to format numbers and dates in PDFDocumentProperties/AnnotationLayer #18638

Conversation

Snuffleupagus
Copy link
Collaborator

@Snuffleupagus Snuffleupagus commented Aug 21, 2024

  • Utilize Fluent to format numbers and dates in PDFDocumentProperties

    The PDFDocumentProperties dialog may not display correctly formatted data, especially in the GENERIC viewer, since it's using native methods[1] that depend on the browser locale instead of the viewer locale as intended.
    At the time when this dialog was introduced that was probably all we could easily do, but with Fluent we're able to improve things since it's got built-in support for formatting numbers and dates. Not only does this simplify the JavaScript code, but it also gives the localizer more fine-grained control of the desired output.

    Please find additional information here:


    [1] toLocaleString, toLocaleDateString, and toLocaleTimeString.

  • Utilize Fluent to format dates in the AnnotationLayer

    The AnnotationLayer may not display correctly formatted data in PopupAnnotations, especially in the GENERIC viewer, since it's using native methods[1] that depend on the browser locale instead of the viewer locale as intended.
    With Fluent we're able to improve things since it's got built-in support for formatting dates. Not only does this simplify the JavaScript code slightly, but it also gives the localizer more fine-grained control of the desired output.

    Please find additional information here:


    [1] toLocaleDateString, and toLocaleTimeString.

@Snuffleupagus Snuffleupagus added viewer l10n Localization labels Aug 21, 2024
@Snuffleupagus Snuffleupagus force-pushed the PDFDocumentProperties-l10n-functions branch 2 times, most recently from cdc500c to c2b922e Compare August 21, 2024 19:52
@Snuffleupagus Snuffleupagus changed the title Utilize Fluent to format numbers and dates in PDFDocumentProperties Utilize Fluent to format numbers and dates in PDFDocumentProperties/AnnotationLayer Aug 21, 2024
@Snuffleupagus Snuffleupagus force-pushed the PDFDocumentProperties-l10n-functions branch from c2b922e to e241a47 Compare August 22, 2024 09:23
@mozilla mozilla deleted a comment from moz-tools-bot Aug 22, 2024
@mozilla mozilla deleted a comment from moz-tools-bot Aug 22, 2024
@mozilla mozilla deleted a comment from moz-tools-bot Aug 22, 2024
@mozilla mozilla deleted a comment from moz-tools-bot Aug 22, 2024
@Snuffleupagus Snuffleupagus force-pushed the PDFDocumentProperties-l10n-functions branch 2 times, most recently from eccc61b to ca3643f Compare August 22, 2024 10:02
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/459191ca401ebd3/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/1eece07fd5dad4a/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/459191ca401ebd3/output.txt

Total script time: 31.34 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 92
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/459191ca401ebd3/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/1eece07fd5dad4a/output.txt

Total script time: 45.49 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 42

Image differences available at: http://54.193.163.58:8877/1eece07fd5dad4a/reftest-analyzer.html#web=eq.log

@Snuffleupagus Snuffleupagus force-pushed the PDFDocumentProperties-l10n-functions branch from ca3643f to f40880c Compare August 23, 2024 19:17
@Snuffleupagus Snuffleupagus marked this pull request as ready for review August 23, 2024 19:21
@Snuffleupagus Snuffleupagus requested a review from a team as a code owner August 23, 2024 19:21
Copy link
Contributor

@flodolo flodolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(with the Swedish changes removed)

l10n/sv-SE/viewer.ftl Outdated Show resolved Hide resolved
@Snuffleupagus Snuffleupagus force-pushed the PDFDocumentProperties-l10n-functions branch from f40880c to 7b36df4 Compare August 25, 2024 09:58
The `PDFDocumentProperties` dialog may not display correctly formatted data, especially in the GENERIC viewer, since it's using native methods[1] that depend on the *browser* locale instead of the viewer locale as intended.
At the time when this dialog was introduced that was probably all we could easily do, but with Fluent we're able to improve things since it's got built-in support for formatting numbers and dates. Not only does this simplify the JavaScript code, but it also gives the localizer more fine-grained control of the desired output.

Please find additional information here:
 - https://projectfluent.org/fluent/guide/builtins.html
 - https://projectfluent.org/fluent/guide/functions.html

---

[1] `toLocaleString`, `toLocaleDateString`, and `toLocaleTimeString`.
The `AnnotationLayer` may not display correctly formatted data in PopupAnnotations, especially in the GENERIC viewer, since it's using native methods[1] that depend on the *browser* locale instead of the viewer locale as intended.
With Fluent we're able to improve things since it's got built-in support for formatting dates. Not only does this simplify the JavaScript code slightly, but it also gives the localizer more fine-grained control of the desired output.

Please find additional information here:
 - https://projectfluent.org/fluent/guide/builtins.html
 - https://projectfluent.org/fluent/guide/functions.html

---

[1] `toLocaleDateString`, and `toLocaleTimeString`.
@Snuffleupagus Snuffleupagus force-pushed the PDFDocumentProperties-l10n-functions branch from 7b36df4 to 6ce9f97 Compare August 25, 2024 10:11
@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.241.84.105:8877/cec4a5ae29fbbda/output.txt

@Snuffleupagus
Copy link
Collaborator Author

@timvandermeij Mind taking a look at the JS-code in the patch, before we land this?
Note that the test "failures" are expected, since we now format English dates slightly differently than before.

@timvandermeij timvandermeij self-requested a review August 25, 2024 10:19
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/cec4a5ae29fbbda/output.txt

Total script time: 1.04 mins

Published

@timvandermeij timvandermeij merged commit cd99be0 into mozilla:master Aug 25, 2024
9 checks passed
@timvandermeij
Copy link
Contributor

Thank you for improving this!

/botio makeref

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_makeref from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/e2a651f3331ab3c/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_makeref from @timvandermeij received. Current queue size: 0

Live output at: http://54.193.163.58:8877/5dabaccbe501dc8/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/e2a651f3331ab3c/output.txt

Total script time: 20.53 mins

  • Make references: Passed
  • Check references: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/5dabaccbe501dc8/output.txt

Total script time: 26.26 mins

  • Make references: Passed
  • Check references: Passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants