RequestPolicyContinued / requestpolicy

a web browser extension that gives you control over cross-site requests. Available for XUL/XPCOM-based browsers.
https://github.com/RequestPolicyContinued/requestpolicy/wiki
Other
252 stars 35 forks source link

Switch locale system to browser.i18n #881

Closed jrrdev closed 6 years ago

jrrdev commented 6 years ago

Changelog

Based on myrdd/feature/we-i18n branch Fixes Issue #873:

Notes

Future enhancements proposals

Some proposals to consider when we will have some spare time (I guess after WE transition):

Are they relevant ? What do you think about it ?

Tests

I performed manual tests on Firefox ESR 52.5.1 Nightly and Firefox 56.0b12 Developer Edition. OS Language was fr_FR (Ubuntu 16.04 32 bits):

$ locale
LANG=fr_FR.UTF-8
LANGUAGE=fr_FR
jrrdev commented 6 years ago

Commit for retrieve default_locale from manifest.json task have been pushed, see https://github.com/RequestPolicyContinued/requestpolicy/pull/881/commits/fc16fae71f86e1f85750d00030d3cd9285f10aa5

myrdd commented 6 years ago

Thank you! I will look into your changes after my vacations. Until then, I will comment on some of your changes. Keep up the great work! 👍👍

jrrdev commented 6 years ago

I added few commits to improve make check-locales command and to make some related fixes. I won't commit anything else beside review corrections to this branch.

myrdd commented 6 years ago

Hi @jrrdev

so far I've made some minor corrections on https://github.com/myrdd/requestpolicy/commits/feature/we-i18n

These are a few notes: (I'm not finished with the review yet, sorry for the delay)

myrdd commented 6 years ago

Add locale message for some hard-coded text (e.g old_rules.html, experimental.html)

Experimental features do not need to be localized IMHO. The old_rules page can be considered to be localized; however, for Firefox 57+ users that page is obsolete. The "old rules" refer to v0.5.x rules, which cannot be accessed by a WE.

I'm going to complete the review now.

jrrdev commented 6 years ago

Hi @jrrdev

so far I've made some minor corrections on https://github.com/myrdd/requestpolicy/commits/feature/we-i18n

These are a few notes: (I'm not finished with the review yet, sorry for the delay)

  • license headers in new GPL files
    • put your name there instead of mine.
    • put there only one year (2017)
  • yourpolicy.js and overlay.js
    • please use const $str = browser.i18n.getMessage.bind(browser.i18n); and then e.g. $str("redirectNotification") to make the code shorter

@myrdd: I merged your corrections in this PR and made the requested changes :wink:

myrdd commented 6 years ago

Hi @jrrdev, I've fixed some issues, made some refactorings and codings style changes, rebased onto and merged into develop. The merge commit is https://github.com/RequestPolicyContinued/requestpolicy/commit/3b2c656dae08da780e96b11c30dd88c637830184. Note that I also changed the directory structure a bit, the api is now at src/conditional/legacy/content/bootstrap/content/models/api/