lingua-libre / SignIt

🌻 Lingua Libre SignIt web-browser extension translates selected word in French Sign Language via an elegant pop up so you learn sign language while reading online.
https://addons.mozilla.org/en-US/firefox/addon/lingua-libre-signit/
MIT License
11 stars 13 forks source link

Minimal test of i18n #86

Closed hugolpz closed 3 weeks ago

hugolpz commented 3 weeks ago

I just read these whole pages (5~7mins) :

Mozilla > Browser extension > Internationalization https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Internationalization#direct_placeholder_usage API : https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/i18n

These are derivated from Chrome.i18n page cited below.

There are no mention of content script vs popup scopes or browser.i18n solving this difficulty. Let's test this live. No need to undo current system, let's first see if i18n works everywhere (modal + popup ; FF + Chrome) in a more resilient way.

Read more :

Test JS

Add the following call to i18n values :

// Somewhere in popup > settings
browser.i18n.getMessage("si_addon_title");
// Somewhere in video gallery
browser.i18n.getMessage("si_panel_videos_gallery_attribution", ['https://link.to/video', 'CreatorName', '2', '4']);

Test manifest.json

In manifest.json, try replacing name's value. Note: maybe the name key's snake_case will mess with that, but need to be tester.

"name": "__MSG_si_addon_title__",

Go further

How to Add Internationalization (i18n) to Your Chrome Extension: Guide (5mins) also provides a step by step migration guide. Importantly, it provides some refresh i18n code sample to learn from.

See also

Results

Scope Chrome Firefox
Popup > Settings βœ” βœ”
Popup > Browse > video gallery βœ” βœ”
Content script > modal > video gallery βœ” βœ”
kabir-afk commented 3 weeks ago

I am currently reading docs , and I feel that the popup UI refresh bug wont be an issue as we wont be relying on local storage then. Another thing I wanted to address is that , many methods under browser.i18n aren't supported in Safari , is that ok ?

hugolpz commented 3 weeks ago

@kabir-afk : Yes, it's ok. We are a Desktop extension as of 2024.

32 and source shows :

Date Browser Market share
2024-03-31 Safari 2.1%

Multiply by x2 to have the share on Desktop, about 4.2. We don't have the human resources to focus on slower players-browsers. We go forward with FF and Chrome (>90% Desktop market share).

kabir-afk commented 3 weeks ago

I also observed that while browser.i18n can substitute values with placeholders , it doesn't additionally parse like banana.i18n did. For example - banan.i18n({{link|$1|$2}}) got parsed into something like <a href="$1">$2</a>. But native API doesn't provide that, so we'd still have to do some labor of our own.

hugolpz commented 3 weeks ago

@kabir-afk hello,

From what I read, with Direct placeholder usage , the following should work :

 "si_panel_videos_gallery_attribution": {
    "message": "by {{link|$1|$2}} – Video $3 of $4",
    "description": ""
  },

together with

browser.i18n.getMessage("si_panel_videos_gallery_attribution", ['https://link.to/video', 'CreatorName', '2', '4']);

Should produce :

by {{link|https://link.to/video|CreatorName}} – Video 2 of 4

Because {{link|url|text}} is a Wikimedia banana.i18n shorthand.

But this is ok ! We can then edit the json files to replace {{link|$1|$2}} by <a href='$1'>$2</a> and it will work fine.

kabir-afk commented 3 weeks ago

Agreed, but by {{link|https://link.to/video|CreatorName}} – Video 2 of 4 is not something we want right ? We want the actual hyperlink. I was just mentioning it , and instead of changing keys in all the files maybe we parse it directly inside SignItVideosGallery.js like we did earlier.

hugolpz commented 3 weeks ago

Let's fully move to full html. This {{link|}} shorthand adds complexity with no gain. Replacement is #88 .

hugolpz commented 3 weeks ago

@kabir-afk, this is good news !

Image

From what I see on branch master, you may want to replace the SignItVideosGallery.prototype.refresh using full browser.i18n code :

+ browser.i18n.getMessage("si_panel_videos_gallery_attribution", [${url}, ${speaker}, index, total]);

Then I believe you may close this issue and we can move to #84.

kabir-afk commented 3 weeks ago

Yea well I have done that already inside the refresh prtotype method. But since we were only testing I thought of keeping the previous banana.i18n methods. Should I remove them from all the other files as well ? Is it finalized that we are migrating from banana.i18n to browser.i18n

hugolpz commented 3 weeks ago

Yes, given your table of results full migration to browser.i18n seems a win. It works everywhere and simplified our i18n approach to one single system.

I close this issue #86, you can push for #84.