Closed
Bug 1453480
Opened 7 years ago
Closed 7 years ago
Update fluent to 0.6.4 and fluent-dom to 0.2.0
Categories
(Core :: Internationalization, enhancement, P3)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: zbraniecki, Assigned: zbraniecki)
References
Details
Attachments
(4 files, 1 obsolete file)
Changelog:
fluent 0.6.4 (April 11, 2018)
Minor optimization to bidirectionality isolation
Added error logging when attempting an already registered message id
fluent-dom 0.2.0
DOM Overlays v2 (#168) Major refactor of DOM Overlays allowing developers to provide node elements as attributes.
Refactored error reporting (#160)
Fixed a minor bug which caused an extranous retranslation when data-l10n-id was being removed from an element.
We care especially for DOM Overlays v2, but will be nice to get the other fixes as well.
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8967155 [details]
Bug 1453480 - Update fluent to 0.6.4 and fluent-dom to 0.2.0.
https://reviewboard.mozilla.org/r/235808/#review241610
Code analysis found 2 defects in this patch:
- 2 defects found by mozlint
You can run this analysis locally with:
- `./mach lint path/to/file` (JS/Python)
If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx
::: intl/l10n/DOMLocalization.jsm:97
(Diff revision 1)
> - templateElement.innerHTML = value;
> - targetElement.appendChild(
> - // The targetElement will be cleared at the end of sanitization.
> - sanitizeUsing(templateElement.content, targetElement)
> );
> + templateElement.innerHTML = value;
Error: Unsafe assignment to innerhtml [eslint: no-unsanitized/property]
::: intl/l10n/DOMLocalization.jsm:655
(Diff revision 1)
> return frag.localize(getTranslationsForItems.bind(this))
> .then(untranslatedElements => {
> for (let i = 0; i < overlayTranslations.length; i++) {
> if (overlayTranslations[i] !== undefined &&
> untranslatedElements[i] !== undefined) {
> overlayElement(untranslatedElements[i], overlayTranslations[i]);
Error: 'overlayelement' is not defined. [eslint: no-undef]
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
The second patch is not yet finished, as it'll require string ID changes. It would be useful to have ftl->ftl migration here to be able to avoid having to make localizers pay the price, but maybe we can save ourselves and migrate from dtd/properties since all those strings should be still in gecko-strings.
Updated•7 years ago
|
Flags: needinfo?(francesco.lodolo)
Assignee | ||
Comment 6•7 years ago
|
||
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8967155 [details]
Bug 1453480 - Update fluent to 0.6.4 and fluent-dom to 0.2.0.
https://reviewboard.mozilla.org/r/235808/#review241724
Attachment #8967155 -
Flags: review?(stas) → review+
Comment 8•7 years ago
|
||
Since we don't have support for FTLtoFTL migrations, we should migrate those strings again from DTD and .properties.
The interesting part is that we now have diffs between what's in FTL, and what would be migrated, because localizers updated only the new FTL files.
I can try to track down the differences, and fix them in Pontoon. The list also highlight one issue: one of the en-US has the wrong apostrophe. One more reason to prioritize bug 1452962 and bug 1416149.
----
Checking browser/browser/preferences/preferences.ftl:search-results-sorry-message
Values are different for my
Found in FTL:
{ PLATFORM() ->
[windows] ဝမ်းနည်းပါတယ်။ အပြင်အဆင်များထဲတွင် “<span></span>” အတွက် ရလဒ်များ မရှိပါ။
*[other] ဝမ်းနည်းပါတယ်။ နှစ်သက်ရာအပြင်အဆင်များထဲတွင် “<span></span>” အတွက် ရလဒ်များ မရှိပါ။
}
Expected from migration:
{ PLATFORM() ->
[windows] ဝမ်းနည်းပါတယ်။ “<span></span>” အတွက် ရွေးစရာများထဲတွင် မရှိပါ။
*[other] ဝမ်းနည်းပါတယ်။ “<span></span>” အတွက် နှစ်သက်ရာအပြင်အဆင်များထဲတွင် မတွေ့ပါ။
}
Checking browser/browser/preferences/preferences.ftl:search-results-need-help
Values are different for ro
Found in FTL:
Ai nevoie de ajutor? Vizitează <a>pagina de suport { -brand-short-name }</a>
Expected from migration:
Ai nevoie de ajutor? Vizitează <a>Suport { -brand-short-name }</a>
Values are different for ka
Found in FTL:
გესაჭიროებათ დახმარება? ეწვიეთ <a>{ -brand-short-name } მხარდაჭერის გვერდს</a>
Expected from migration:
გესაჭიროებათ დახმარება? იხილეთ <a>{ -brand-short-name } მხარდაჭერის გვერდი</a>
Checking browser/browser/preferences/preferences.ftl:update-application-info
Values are different for hi-IN
Found in FTL:
संस्करण{ $version } <a>क्या नया है?</a>
Expected from migration:
संस्करण{ $version } <a>क्या नया है?</a>
Values are different for en-US
Found in FTL:
Version { $version } <a>What's new</a>
Expected from migration:
Version { $version } <a>What’s new</a>
Checking browser/browser/preferences/preferences.ftl:performance-limit-content-process-disabled-desc
Values are different for lij
Found in FTL:
Cangiâ o numero de cntegnui de processo o l'é poscibile solo in { -brand-short-name } moltiprocesso. <a>Amia comme controlâ se o moltiprocesso o l'é ativo</a>
Expected from migration:
Se peu cangiâ o limite de numero de contegnui de processo solo se ti gh'æ o { -brand-short-name } moltiprocesso. <a>Amia comme se fâ a vedde se o moltiprocesso o l'é abilitou</a>
Checking browser/browser/preferences/preferences.ftl:tracking-description
Values are different for pl
Found in FTL:
Ochrona przed śledzeniem blokuje elementy, które zbierają informacje o przeglądaniu na wielu różnych stronach. <a>Więcej informacji o ochronie przed śledzeniem i prywatności</a>.
Expected from migration:
Ochrona przed śledzeniem blokuje elementy, które zbierają informacje o przeglądaniu na wielu różnych stronach. <a>Więcej informacji o ochronie przed śledzeniem i prywatności</a>
Flags: needinfo?(francesco.lodolo)
Comment 9•7 years ago
|
||
I initially tried to fix Polish, but realized the result would be different (period outside of link) and backed it out
https://hg.mozilla.org/l10n-central/pl/rev/9a1cd7fa8d7e
All other differences should be fixed, keeping the most recent one (in FTL).
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8967207 [details]
Bug 1453480 - Migrate Fluent resources to use DOM Overlays v2.
https://reviewboard.mozilla.org/r/235874/#review241766
::: browser/locales/en-US/browser/preferences/preferences.ftl:83
(Diff revision 1)
>
> ## Preferences UI Search Results
>
> search-results-header = Search Results
>
> # `<span></span>` will be replaced by the search term.
Need to update the comment
::: browser/locales/en-US/browser/preferences/preferences.ftl:270
(Diff revision 1)
>
> update-application-title = { -brand-short-name } Updates
>
> update-application-description = Keep { -brand-short-name } up to date for the best performance, stability, and security.
>
> -update-application-info = Version { $version } <a>What's new</a>
> +update-application-info = Version { $version } <a data-l10n-name="learn-more">What's new</a>
What’s new (apostrophe)
Comment 11•7 years ago
|
||
Assuming we rename string ID adding a 2, otherwise needs updating.
Updated•7 years ago
|
Attachment #8967330 -
Attachment is patch: true
Attachment #8967330 -
Attachment mime type: text/x-python-script → text/plain
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 14•7 years ago
|
||
Comment on attachment 8967330 [details] [diff] [review]
bug_1453480_preferences_dom2_resources.py
Thank you Flod for the migration! I adjusted it and changed the IDs to be more meaningful. It's ready for review :)
Attachment #8967330 -
Attachment is obsolete: true
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8967207 [details]
Bug 1453480 - Migrate Fluent resources to use DOM Overlays v2.
https://reviewboard.mozilla.org/r/235874/#review241936
Looks good, thanks
Attachment #8967207 -
Flags: review?(francesco.lodolo) → review+
Comment 16•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/385de3e4dca0
Update fluent to 0.6.4 and fluent-dom to 0.2.0. r=stas
https://hg.mozilla.org/integration/autoland/rev/6b5e7c13eb8c
Migrate Fluent resources to use DOM Overlays v2. r=flod
Comment 17•7 years ago
|
||
Backed out 3 changesets (bug 1453480, bug 1453878) for c2 failures at intl/l10n/test/dom/test_domloc_overlay.htm and browser-chrome failures at browser/components/preferences/in-content/tests/browser_fluent.js on a CLOSED TREE
Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=6b5e7c13eb8c7c77dec98f0eec17b4a26fdbf060
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=173419656&repo=autoland&lineNumber=6078
Backout: https://hg.mozilla.org/integration/autoland/rev/de2f038f711e379b9ac4b7eb85fb5387b532ffc7
Flags: needinfo?(gandalf)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•7 years ago
|
||
Comment on attachment 8967155 [details]
Bug 1453480 - Update fluent to 0.6.4 and fluent-dom to 0.2.0.
So, there's more. In the new overlays, stas changed `attrs` to `attributes` which requires updates to our Node.localize and Preferences calls.
I'd also like to re-request r? on the first chunk because I fixed some tests and ported the updated `messagesFromContext` (not sure how I missed it during the first migration, but now I tested and I'm confident it was the only thing I didn't include).
Flags: needinfo?(gandalf)
Attachment #8967155 -
Flags: review+ → review?(stas)
Comment 23•7 years ago
|
||
mozreview-review |
Comment on attachment 8967155 [details]
Bug 1453480 - Update fluent to 0.6.4 and fluent-dom to 0.2.0.
https://reviewboard.mozilla.org/r/235808/#review242144
Should this also update intl/l10n/fluent.js.patch?
Attachment #8967155 -
Flags: review?(stas) → review+
Comment 24•7 years ago
|
||
mozreview-review |
Comment on attachment 8967648 [details]
Bug 1453480 - Update Preferences to use fluent-dom 0.2.0 API.
https://reviewboard.mozilla.org/r/236384/#review242146
::: browser/components/preferences/in-content/findInPage.js:486
(Diff revision 1)
> if (!msg) {
> console.error(`Missing search l10n id "${refId}"`);
> return null;
> }
> if (refAttr) {
> - let attr = msg.attrs.find(a => a.name === refAttr);
> + let attr = msg.attributes.find(a => a.name === refAttr);
`attributes` may be null if the message doesn't have any.
Attachment #8967648 -
Flags: review?(stas) → review-
Comment 25•7 years ago
|
||
mozreview-review |
Comment on attachment 8967649 [details]
Bug 1453480 - Update Node.localize API to renamed attrs->attributes in fluent-dom 0.2.0.
https://reviewboard.mozilla.org/r/236386/#review242184
Attachment #8967649 -
Flags: review?(bugs) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 28•7 years ago
|
||
> Should this also update intl/l10n/fluent.js.patch?
Good call, I'll add it before landing.
Comment 29•7 years ago
|
||
mozreview-review |
Comment on attachment 8967648 [details]
Bug 1453480 - Update Preferences to use fluent-dom 0.2.0 API.
https://reviewboard.mozilla.org/r/236384/#review242222
Attachment #8967648 -
Flags: review?(stas) → review+
Comment 30•7 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #28)
> > Should this also update intl/l10n/fluent.js.patch?
>
> Good call, I'll add it before landing.
Thanks. In general, I think it might be useful to review fluent.js.patch rather than the other files. If you're taking them from fluent.js's master, they've already been reviewed :)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 39•7 years ago
|
||
Comment 40•7 years ago
|
||
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/658fedb903d0
Update fluent to 0.6.4 and fluent-dom to 0.2.0. r=stas
https://hg.mozilla.org/integration/autoland/rev/46a634d6853c
Migrate Fluent resources to use DOM Overlays v2. r=flod
https://hg.mozilla.org/integration/autoland/rev/a3c36fa7ac0c
Update Preferences to use fluent-dom 0.2.0 API. r=stas
https://hg.mozilla.org/integration/autoland/rev/623b37fe0fe8
Update Node.localize API to renamed attrs->attributes in fluent-dom 0.2.0. r=smaug
Comment 41•7 years ago
|
||
Backed out 4 changesets (bug 1453480) for failing browser-chrome at browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js on a CLOSED TREE
Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=623b37fe0fe82d6eff6634cda655252f95b4f8b1
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=173543451&repo=autoland&lineNumber=6941
Backout: https://hg.mozilla.org/integration/autoland/rev/bab10eeb6799a277b7237851293041cbec55a370
[task 2018-04-13T16:24:08.387Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.388Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.390Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.391Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.394Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.395Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.396Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.399Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.401Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.402Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.405Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.407Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.408Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.409Z] 16:24:08 INFO - Buffered messages finished
[task 2018-04-13T16:24:08.410Z] 16:24:08 INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should be in search results -
[task 2018-04-13T16:24:08.411Z] 16:24:08 INFO - Stack trace:
[task 2018-04-13T16:24:08.413Z] 16:24:08 INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/head.js:is_element_visible:24
[task 2018-04-13T16:24:08.415Z] 16:24:08 INFO - chrome://mochitests/content/browser/browser/components/preferences/in-content/tests/head.js:evaluateSearchResults:160
[task 2018-04-13T16:24:08.416Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.417Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should be in search results -
[task 2018-04-13T16:24:08.418Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.419Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.420Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.421Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.422Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.423Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.424Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.425Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
[task 2018-04-13T16:24:08.426Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Element should not be null, when checking visibility -
[task 2018-04-13T16:24:08.427Z] 16:24:08 INFO - TEST-PASS | browser/components/preferences/in-content/tests/browser_search_subdialogs_within_preferences_site_data.js | Should not be in search results -
Flags: needinfo?(gandalf)
Comment 42•7 years ago
|
||
This bug did not cause the above mentioned failure, relading it. We apologize for the confusion.
Flags: needinfo?(gandalf)
Comment 43•7 years ago
|
||
Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e11e552e28c5
Update fluent to 0.6.4 and fluent-dom to 0.2.0. r=stas
https://hg.mozilla.org/integration/autoland/rev/54886ee3bc50
Migrate Fluent resources to use DOM Overlays v2. r=flod
https://hg.mozilla.org/integration/autoland/rev/4de9bbcfd440
Update Preferences to use fluent-dom 0.2.0 API. r=stas
https://hg.mozilla.org/integration/autoland/rev/b0f73dc15179
Update Node.localize API to renamed attrs->attributes in fluent-dom 0.2.0. r=smaug on a CLOSED TREE
Comment 44•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e11e552e28c5
https://hg.mozilla.org/mozilla-central/rev/54886ee3bc50
https://hg.mozilla.org/mozilla-central/rev/4de9bbcfd440
https://hg.mozilla.org/mozilla-central/rev/b0f73dc15179
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•