1814168 - Report on the download status and progress in about:translations

Status: RESOLVED FIXED

(Firefox :: Translations, enhancement, P3)

Description

[Greg Tatum [:gregtatum]]

The wasm and language models are downloaded from Remote Settings. Right now things are just dimmed out when we don't have the data, but we should communicate to the user when this isn't available yet.

The addon has a bunch of verbose messages that report the state to the user, and displays the download percentages. I don't believe that Remote Settings surfaces the download percentages at this time, so they may need to be added if we want to surface this to the UI.

Comment 1

[Greg Tatum [:gregtatum]]

Here you can see it taking 12 seconds to download a model, which is very confusing for a user:

https://profiler.firefox.com/public/vmfpp10rk75p0g9dxd9ma2c53kstqsq74zmv168/network-chart/?globalTrackOrder=9wb1w6078&hiddenGlobalTracks=1w6&hiddenLocalTracksByPid=183492-03w8~183830-0~183673-0~184027-0~184082-0~183696-0~183962-0~183717-0&thread=0&v=8

Comment 2

[Emilio Cobos Álvarez [:emilio]]

I was about to file this. I thought about:translations didn't work at all (on Linux, too fwiw)

Comment 3

[Greg Tatum [:gregtatum]]

The un-privileged UI code for this lives in toolkit/components/translations/content/translations.mjs

The remote settings code is privileged code, and needs to be wired in using the AboutTranslationsChild actor. This is done with a global function similar to this. Or maybe it would be fine to just have a spinner awaiting the promise to download the model and the engine. This is what we do in the URL bar.

about:translations is currently in our backlog and is a developer-focused tool for now for testing the translations engine.

Comment 4

[anayocrescent2]

please can i get more details on this bug

Comment 5

[Thierry]

Please, could this task be assigned to me ?

Comment 6

[Marco Castelluccio [:marco]]

Feel free to work on any good first bug that is currently open and unassigned. Bugs are assigned once there is a patch ready and attached to the bug.

Comment 7

[Thierry]

Can you give me more informations about this bug

Comment 8

[Thierry]

(In reply to Thierry from comment #7)

Can you give me more information about this bug

Comment 9

[Abdulsamad]

(In reply to Greg Tatum [:gregtatum] from comment #3)

The un-privileged UI code for this lives in toolkit/components/translations/content/translations.mjs

The remote settings code is privileged code, and needs to be wired in using the AboutTranslationsChild actor. This is done with a global function similar to this. Or maybe it would be fine to just have a spinner awaiting the promise to download the model and the engine. This is what we do in the URL bar.

about:translations is currently in our backlog and is a developer-focused tool for now for testing the translations engine.

@gregtatum @nordzilla do you mean it's just fine for a spinner to be input just before creating the translation engine?
what I realized going through the code is that there are different instances of creating the translation engine.
it looks to me like the translation engine is being destroyed and recreated after a change in language selection,
OR is the spinner to be before the entire page even loads. cause you said something about a dimming feature.
I realized when i used the "about:translations", no dimming feature was apparent to me.
id appreciate if you could shed light on this so i could know if im on the right path
Thank you
abd007
(outreachy)

Comment 10

[peterodejobi9]

(In reply to Greg Tatum [:gregtatum] from comment #3)

The un-privileged UI code for this lives in toolkit/components/translations/content/translations.mjs

The remote settings code is privileged code, and needs to be wired in using the AboutTranslationsChild actor. This is done with a global function similar to this. Or maybe it would be fine to just have a spinner awaiting the promise to download the model and the engine. This is what we do in the URL bar.

about:translations is currently in our backlog and is a developer-focused tool for now for testing the translations engine.

Please what are you referring to when you say the model ? is it the translationRequest ?

Comment 11

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implement Loading Spinner for Download Progress in about:translations. r=gregtatum,nordzilla!

Comment 12

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implement Loading Spinner for Download Progress in about:translations and fixed lint error. r=gregtatum,nordzilla!

Depends on D191027

Comment 13

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implement Loading Spinner for Download Progress in about:translations and fixed lint error. r=gregtatum,nordzilla!

Depends on D191027

Comment 14

[Erik Nordin [:nordzilla]]

Hi peterodejobi9

It looks like you've got 3 patches submitted for the exact same behavior.

Could you please take the time to consolidate everything locally into a single patch and then abandon the other two?

To abandon a patch, go to the patch in Phabricator, scroll all the way to the bottom and choose "Add Action..." and abandon the revision.

It looks like they are also linked together as parent/child revisions. It would be good to unlink them from each other and only keep the one patch that you intend to have reviewed.

To unlink them, go to the top of the page in Phabricator, on the right side, and use the "Edit Related Revisions" option.

Comment 15

[peterodejobi9]

(In reply to Erik Nordin [:nordzilla] from comment #14)

Hi peterodejobi9

It looks like you've got 3 patches submitted for the exact same behavior.

Could you please take the time to consolidate everything locally into a single patch and then abandon the other two?

To abandon a patch, go to the patch in Phabricator, scroll all the way to the bottom and choose "Add Action..." and abandon the revision.

It looks like they are also linked together as parent/child revisions. It would be good to unlink them from each other and only keep the one patch that you intend to have reviewed.

To unlink them, go to the top of the page in Phabricator, on the right side, and use the "Edit Related Revisions" option.

Hi Erick one of the related patches is for the linting error phab-bot complained about.

Comment 16

[peterodejobi9]

(In reply to Erik Nordin [:nordzilla] from comment #14)

Hi peterodejobi9

It looks like you've got 3 patches submitted for the exact same behavior.

Could you please take the time to consolidate everything locally into a single patch and then abandon the other two?

To abandon a patch, go to the patch in Phabricator, scroll all the way to the bottom and choose "Add Action..." and abandon the revision.

It looks like they are also linked together as parent/child revisions. It would be good to unlink them from each other and only keep the one patch that you intend to have reviewed.

To unlink them, go to the top of the page in Phabricator, on the right side, and use the "Edit Related Revisions" option.

Thank you Eric I'll get to work on everything you said right now.

Comment 17

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implement Loading Spinner for Download Progress in about:translations. r=gregtatum,nordzilla!

Comment 18

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Added loading spinner for when the worker is rebuilt i.e language pairs change.r=gregtatum,nordzilla!

is rebuilts i.e language pairs change. r=gregtatum,nordzilla!

when the worker is rebuilt i.e when language pairs change.r=gregtatum,nordzilla!

Depends on D191143

Comment 19

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Added loading spinner for when the worker is rebuilt i.e language pairs change.r=gregtatum,nordzilla!

Depends on D191258

Comment 20

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Added loading spinner for when the worker is rebuilt i.e language pairs change.r=gregtatum,nordzilla!

Depends on D191261

Comment 21

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Added loading spinner for when the worker is rebuilt i.e when the language pairs change.r=gregtatum,nordzilla!

Depends on D191258

Comment 22

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Added loading spinner for when worker is rebuilt i.e when the language pairs change.r=gregtatum,nordzilla!

Depends on D191143

Comment 23

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implemented Loading Spinner for Download Progress and worker rebuild i.e when language pair change.r=gregtatum,nordzilla!

Depends on D191143

Comment 24

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implemented Loading Spinner for Download Progress and worker rebuild i.e when language pair change.r=gregtatum,nordzilla!

Depends on D191331

Comment 25

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implemented Loading Spinner for Download Progress and worker rebuild i.e when language pair change.r=gregtatum,nordzilla!

Depends on D191334

Comment 26

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implemented Loading Spinner for Download Progress and worker rebuild i.e when language pair change.r=gregtatum,nordzilla!

Depends on D191336

Comment 27

[Greg Tatum [:gregtatum]]

Hey rinwaoluwa, could you please not create so many attachments and mark them as obsolete? I'm not sure what you're trying to do, but it's not the proper way to update revisions. You can push updates to an existing revision on Phabricator. I'm getting an email for every action you are taking here, and it's starting to be a bit disruptive. In addition CI tasks run against the revisions when you push them up. You should only push up updates when you are ready for additional feedback.

Comment 28

[peterodejobi9]

(In reply to Greg Tatum [:gregtatum] from comment #27)

Hey rinwaoluwa, could you please not create so many attachments and mark them as obsolete? I'm not sure what you're trying to do, but it's not the proper way to update revisions. You can push updates to an existing revision on Phabricator. I'm getting an email for every action you are taking here, and it's starting to be a bit disruptive. In addition CI tasks run against the revisions when you push them up. You should only push up updates when you are ready for additional feedback.

Hi Erick i'm so sorry about that i just pushed the finished and tested code and abandoned the rest of the revision i was trying to make all the changes into 1 patch like you requested the last time and i have finalized that. I'm sorry once again it's my first time working with mecurial i was having a hard time with it.

Comment 29

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implemented Loading Spinner for Download Progress and worker rebuild i.e when language pair change.r=gregtatum,nordzilla!

Comment 30

[peterodejobi9]

Attachment: Bug 1814168 - [Browser UI] Implemented Loading Spinner for Download Progress and worker rebuild i.e when language pair change,also added a test file. r=nordzilla!

Comment 31

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 32

[Gabriel Lee [:gabrielchl]]

Attachment: Bug 1814168 - Show translating message when loading translations. r=#translations-reviewers

r=translations-reviewers

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f04132de4d4a
Show translating message when loading translations. r=translations-reviewers,fluent-reviewers,nordzilla

Comment 34

[tszentpeteri]

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