1948995 - Allow Full-Page Translations on moz-extension URLs

Status: RESOLVED FIXED

(Firefox :: Translations, task)

Description

[Erik Nordin [:nordzilla]]

Description

After discussing with community members and some of the Mozilla Addons folks, we have decided that Full Page Translations should be supported on moz-extension:// URLs.

This has the potential to be a good first bug, since the primary updates are not around core, critical functionality, but around allow-list behavior. It should be relatively few lines of code. Note, however, that there may be complexities in setting up automated testing for this bug that are not as simple. I'm going to go ahead and mark this as good-next-bug instead of good-first-bug.

---

Steps to Implement

1. You should first verify that Full Page Translations is not available in the current state of their local Firefox build. I recommend manual testing with the the Remote Settings Devtools extension. You can download the remote-settings-devtools.xpi file from the releases page. Once installed, you can open the extension page by clicking on the puzzle-piece button near the top-right of the browser. Once open, you can access Full Page Translations by clicking the three-line Application Menu directly next to the extensions button. You should see that the "Translate page..." option is disabled.

2. You will have to update which URL schemes are allowed within the TranslationsParent JS actor, adding moz-extension to the list of allowed schemes. Take careful note of the comments within this function, since there is another place within the code base where this logic will have to be updated.

3. If done correctly, you should be able to rebuild and launch Firefox, then test that you now have access to translate the extension page.

4. At this point the implementation is complete. It's always a good idea to run the full Translations test suite to ensure that you haven't regressed anything.

❯ python3 toolkit/components/translations/tests/scripts/download-translations-artifacts.py

(Then follow the next steps outline by this script's output.)

❯ ./mach test --headless toolkit/components/translations/tests browser/components/translations/tests

Ideally this change has not regressed any tests in our test suite.

---

Tests to Implement

Now that it's working, we still can't land the functionality until it is tested.

You will have to add a new test page to the Translations test suite that uses a moz-extension URL. I have not personally done this before, and it may prove to have some hidden complexity compared to the implementation portion.

At a glance, it looks like you may be able to import ExtensionTestUtils like so. You will want to add this to the rest of our imports in shared-head.js.

You would then need to look into getting an actual moz-extension:// page to load in a mochitest within the Translations test suite.

Alternatively, if that proves to be difficult, we could try adding a test to an area of the code base that already tests extension pages, and instead trigger the full-page translation there.

Comment 1

[Brian Ouyang]

Attachment: Bug 1948995 - Allow Full-Page Translations on moz-extension URLs. r?nordzilla!

Comment 2

[Brian Ouyang]

Hi, I am a newer contributor and working on this bug but could use some guidance on the "Tests to Implement" step.

The changes to the URL schemes successfully allow full-page translations for moz-extension pages. I ran the full Translations test suite and the tests passed.

Right now I am trying to understand implementing new tests. From exploring the files and test suite, it seems like:

• toolkit/components/translations/tests contains tests related to translation logic
• browser/components/translations/tests contains tests related to UI and browser-specifc translation.

Based on this, it seems like I would want to add tests to browser/components/translations/tests/browser since the change is focused on ensuring translation functionality works on moz-extension:// pages, rather than testing the underlying translation logic. Does that sound correct?

I attempted to add a test by copying an existing test file and running it with the following command:
./mach test browser/components/translations/tests/browser/new_file_name.js

However, I received the following error, indicating that simply adding a file to the folder isn’t the correct way to “add tests” to the suite:

UNKNOWN TEST: browser/components/translations/tests/browser/new_file_name.js
I was unable to find tests from the given argument(s).

You should specify a test directory, filename, test suite name, or
abbreviation.

It's possible my little brain doesn't know about the type of test you are
trying to execute. If you suspect this, please request support by filing
a bug at
https://bugzilla.mozilla.org/enter_bug.cgi?product=Testing&component=General.

I found documentation on mochitest about adding tests to the tree but wanted to confirm that I was on the right track before doing anything further. Would this be the correct way to add tests?

Lastly, regarding which tests to add: there are already a lot of tests for translations on regular pages. Should we essentially repeat those tests for moz-extension:// pages? Or is there a way to include moz-extension:// pages in the same tests that run for regular pages, ie. is there an easier way where we run tests on both xyz.html and moz-extension://xyz in the same test?

Thanks in advance!

Comment 3

[Erik Nordin [:nordzilla]]

Hey Brian,

This is really great! Thank you for taking this on and asking such a detailed question.

---

Right now I am trying to understand implementing new tests. From exploring the files and test suite, it seems like:

• toolkit/components/translations/tests contains tests related to translation logic
• browser/components/translations/tests contains tests related to UI and browser-specifc translation.

Based on this, it seems like I would want to add tests to browser/components/translations/tests/browser since the change is focused on ensuring translation functionality works on moz-extension:// pages, rather than testing the underlying translation logic. Does that sound correct?

Your observations here are very close.

The toolkit path is actually the core functionality that is shared across all of Gecko. This is the functionality that is shared across Firefox for Desktop, Firefox for Android, and even Thunderbird if Thunderbird adds a translation feature at any point. As such, a lot of the tests for our core logic lie here, as you noticed.

The browser path is specific to Firefox for Desktop, which is where a lot of our UI and browser-specific translation happens.

---

Regarding adding a test, the documentation you found is correct.

Your test likely isn't recognized because it's not added to the browser.toml file where tests are registered.

So what you'll want to run is

❯ ./mach addtest browser/components/translations/tests/browser/new_file_name.js

Which should configure everything correctly for you. Note that if the file exists this will fail, so you will have to temporarily remove your file and then move the contents back in to the file that this command creates.

---

Regarding the testing itself, you're actually in luck. We have an in-progress patch that will be landing soon, that actually already checks the availability of the Translations actor in extension pages.

https://phabricator.services.mozilla.com/D241099#inline-1334210

I expect this patch to land soon, however in the meantime you may want to base your stack on top of this patch.

You can run the following command to pull this patch down:

❯ moz-phab patch D241099

Then you can add your commit on top of that.

Now, when you run the translations tests,

❯ ./mach test --headless toolkit/components/translations/tests browser/components/translations/tests

this test case will actually fail:

Overall Summary
===============

mochitest-browser
~~~~~~~~~~~~~~~~~
Ran 38674 checks (38517 subtests, 157 tests)
Expected results: 38670
Skipped: 3 tests
Unexpected results: 1
  subtest: 1 (1 fail)

Unexpected Results
------------------
toolkit/components/translations/tests/browser/browser_translations_actor_availability.js
  FAIL moz-extension:-page in tab does not have actor - false == true - 
Stack trace:
chrome://mochitests/content/browser/toolkit/components/translations/tests/browser/browser_translations_actor_availability.js:test_actor_at_moz_extension:83
chrome://mochikit/content/browser-test.js:handleTask:1170
chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1242
chrome://mochikit/content/browser-test.js:Tester_execTest:1383
chrome://mochikit/content/browser-test.js:nextTest/<:1159
chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058

So you will have to update the test expectation as part of this patch.

This test only tests for the actor's availability on the page, so we should still probably write a browser test that attempts to trigger the translation on an extension page.

The nice thing is that you can use this test case as an example of how to load your own extension page. This example didn't exist when I filed the bug initially or I would have pointed to it.

---

Let me know if all of this is clear, and please don't hesitate to reach back out if you have other questions.

Thank you for submitting a WIP patch and taking on this task!

Comment 4

[keanucuco]

(In reply to Brian Ouyang from comment #2)

Hi, I am a newer contributor and working on this bug but could use some guidance on the "Tests to Implement" step.

The changes to the URL schemes successfully allow full-page translations for moz-extension pages. I ran the full Translations test suite and the tests passed.

Right now I am trying to understand implementing new tests. From exploring the files and test suite, it seems like:

• toolkit/components/translations/tests contains tests related to translation logic
• browser/components/translations/tests contains tests related to UI and browser-specifc translation.

Based on this, it seems like I would want to add tests to browser/components/translations/tests/browser since the change is focused on ensuring translation functionality works on moz-extension:// pages, rather than testing the underlying translation logic. Does that sound correct?

I attempted to add a test by copying an existing test file and running it with the following command:
./mach test browser/components/translations/tests/browser/new_file_name.js

However, I received the following error, indicating that simply adding a file to the folder isn’t the correct way to “add tests” to the suite:

UNKNOWN TEST: browser/components/translations/tests/browser/new_file_name.js
I was unable to find tests from the given argument(s).

You should specify a test directory, filename, test suite name, or
abbreviation.

It's possible my little brain doesn't know about the type of test you are
trying to execute. If you suspect this, please request support by filing
a bug at
https://bugzilla.mozilla.org/enter_bug.cgi?product=Testing&component=General.

I found documentation on mochitest about adding tests to the tree but wanted to confirm that I was on the right track before doing anything further. Would this be the correct way to add tests?

Lastly, regarding which tests to add: there are already a lot of tests for translations on regular pages. Should we essentially repeat those tests for moz-extension:// pages? Or is there a way to include moz-extension:// pages in the same tests that run for regular pages, ie. is there an easier way where we run tests on both xyz.html and moz-extension://xyz in the same test?

Thanks in advance!

Hi Brian I've also been looking at this bug, you're right browser/components/translations/tests/browser is the right place to add this test. Also i dont think we need to repeat all the tests as that would create a lot of duplicate code and not be fun to deal with because:
we're only changing which URLs can access translations, not how translation works and the existing tests already verify the translation functionality itself.

As for the test errors you're getting in addition to making a new test file its also necessary to add an entry of it into the test manifest. That would be browser.toml in browser/components/translations/tests/browser/

If you want to run the full translations test suite (including your new_file_name.js test), don't forget to download the translation artifacts first:

run python3 toolkit/components/translations/tests/scripts/download-translations-artifacts.py

Then follow the instructions this script outputs before running the tests:

run ./mach test --headless toolkit/components/translations/tests browser/components/translations/tests
These translation artifacts are needed because the tests use actual translation models to verify functionality

hopefully this helps, and if im missing anything @nordzilla can hopefully help us both

Comment 5

[keanucuco]

(In reply to keanucuco from comment #4)

(In reply to Brian Ouyang from comment #2)

Hi, I am a newer contributor and working on this bug but could use some guidance on the "Tests to Implement" step.

The changes to the URL schemes successfully allow full-page translations for moz-extension pages. I ran the full Translations test suite and the tests passed.

Right now I am trying to understand implementing new tests. From exploring the files and test suite, it seems like:

• toolkit/components/translations/tests contains tests related to translation logic
• browser/components/translations/tests contains tests related to UI and browser-specifc translation.

Based on this, it seems like I would want to add tests to browser/components/translations/tests/browser since the change is focused on ensuring translation functionality works on moz-extension:// pages, rather than testing the underlying translation logic. Does that sound correct?

I attempted to add a test by copying an existing test file and running it with the following command:
./mach test browser/components/translations/tests/browser/new_file_name.js

However, I received the following error, indicating that simply adding a file to the folder isn’t the correct way to “add tests” to the suite:

UNKNOWN TEST: browser/components/translations/tests/browser/new_file_name.js
I was unable to find tests from the given argument(s).

You should specify a test directory, filename, test suite name, or
abbreviation.

It's possible my little brain doesn't know about the type of test you are
trying to execute. If you suspect this, please request support by filing
a bug at
https://bugzilla.mozilla.org/enter_bug.cgi?product=Testing&component=General.

I found documentation on mochitest about adding tests to the tree but wanted to confirm that I was on the right track before doing anything further. Would this be the correct way to add tests?

Lastly, regarding which tests to add: there are already a lot of tests for translations on regular pages. Should we essentially repeat those tests for moz-extension:// pages? Or is there a way to include moz-extension:// pages in the same tests that run for regular pages, ie. is there an easier way where we run tests on both xyz.html and moz-extension://xyz in the same test?

Thanks in advance!

Hi Brian I've also been looking at this bug, you're right browser/components/translations/tests/browser is the right place to add this test. Also i dont think we need to repeat all the tests as that would create a lot of duplicate code and not be fun to deal with because:
we're only changing which URLs can access translations, not how translation works and the existing tests already verify the translation functionality itself.

As for the test errors you're getting in addition to making a new test file its also necessary to add an entry of it into the test manifest. That would be browser.toml in browser/components/translations/tests/browser/

If you want to run the full translations test suite (including your new_file_name.js test), don't forget to download the translation artifacts first:

run python3 toolkit/components/translations/tests/scripts/download-translations-artifacts.py

Then follow the instructions this script outputs before running the tests:

run ./mach test --headless toolkit/components/translations/tests browser/components/translations/tests
These translation artifacts are needed because the tests use actual translation models to verify functionality

hopefully this helps, and if im missing anything @nordzilla can hopefully help us both

I unfortunately missed nordzilla's new comment it addreses mostly everything, i hope at least ''run python3 toolkit/components/translations/tests/scripts/download-translations-artifacts.py'' this helps, though with the new patch im not sure we still need to run it

Comment 6

[Brian Ouyang]

Thanks Erik for the clarification and guidance! I really appreciate you taking the time to mentor me on this bug.

I was able to modify the failing test in browser_translations_actor_availability.js to be aligned with moz-extension pages given the new update.

I am running into a few roadblocks again unfortunately with writing the new browser test. I had a couple different ideas that wanted to get feedback on (code is in the WIP Phabricator page for reference and am calling this test file browser_translations_moz_extension_translate_page.js)

Current Approach

I found some other tests like this one browser_translations_select_panel_translate_full_page_button.js which seem to accomplish the "Translate Page" activity but on a regular page:

• My approach was to load a moz-extension page instead of a regular page and trigger the translate page and open the translation panel. However, if I try to press the button, it tries to download and then the test times out. For example:

 0:10.44 GECKO(12890) console.log: Translations: "Beginning to translate." "moz-extension://5697fae3-cfa9-4497-911c-d4a5338289cd/test_page.html"
 0:10.45 GECKO(12890) console.log: Translations: "Creating a new engine for \"es -> en\"."
 0:10.45 GECKO(12890) console.log: Translations: "Beginning model downloads: \"es\" to \"en\""
 0:10.45 GECKO(12890) console.log: Download requested: test-translation-models-0 vocab.esen.spm
 0:10.45 GECKO(12890) console.log: Download requested: test-translation-models-0 model.esen.intgemm.alphas.bin
 0:10.45 GECKO(12890) console.log: Translations: "Getting remote bergamot-translator wasm records."
 0:10.45 GECKO(12890) console.log: Translations: "Using bergamot-translator@undefined release version 2.0 first released on Fxundefined" ({id:"15d83457-90aa-42ed-b329-64aa8f7a3db6", name:"bergamot-translator", version:"2.0", last_modified:1743268953197, schema:1743268953197})
 0:10.45 GECKO(12890) console.log: Download requested: test-translation-wasm-1 bergamot-translator
 0:14.90 GECKO(12890) console.log: Translations: "Pausing translations with pending translation requests."
 0:14.91 GECKO(12890) console.log: Translations: "Discarding translations, innerWindowId:" 4294967302
 0:14.91 GECKO(12890) console.log: Translations: "Force shutdown of the engine \"es -> en\""

• This is where my WIP code is currently at and I haven't been able to figure out how to resolve this. Although overall, I'd imagine that what I'm doing is not the preferred way of testing this functionality as it depends on getting DOM elements from hardcoded id's which could change in the future.

Alternate approaches:

1. Using your suggestion of using the browser_translations_actor_availability.js file as a template of loading a moz-extension page. I was able to load this in a tab but my issue here is that I am not sure how to interact with the tab object to either open the sidebar or trigger a "Translate Page". This seems like it would be similar to my current approach of loading the moz-extension page.

2. I also saw that there are a lot of tests in browser/components/extensions/test/browser that might provide a better template for interacting with moz-extension pages. Would it make sense to use one of these as a starting point for my test?

I'd imagine that there are some Test Utils out there that could simplify this process but could use a little nudge in the right direction on where to look. Thanks again!

Comment 7

[Erik Nordin [:nordzilla]]

I've responded to your comment in D242645.

Please review the comments on Phabricator.

Comment 8

[John Mfon Inyang]

good day, please can i work on this ?

Comment 9

[John Mfon Inyang]

Attachment: Bug 1948995 - Add browser test for translation feature on moz-extension URL r=nordzilla!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bb946266ed2
Allow Full-Page Translations on moz-extension URLs. r=nordzilla,translations-reviewers

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/9bb946266ed2

Comment 12

[Erik Nordin [:nordzilla]]

Brian, we are celebrating the successful landing of your code over in the #Firefox Translations Matrix channel.

This is a feature that was requested by a community member, and then implemented by a community member. I think that's amazing.

Feel free to stop by and say hi if you would like.

• https://wiki.mozilla.org/Matrix

Comment 13

[Erik Nordin [:nordzilla]]

Release Note Request (optional, but appreciated)

[Why is this notable]:

• Full-Page Translations is now available on moz-extension:// pages.

[Affects Firefox for Android]:

• Yes

[Suggested wording]:

• Full-Page Translations is now available within Firefox extension pages.

[Links (documentation, blog post, etc)]:

• N/A

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Added to the Fx139 relnotes. Might be nice if we could get a screenshot showing this in action before go-live.