Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable chromium translation engine #18593

Closed
atuchin-m opened this issue Oct 6, 2021 · 5 comments · Fixed by brave/brave-core#10453
Closed

Enable chromium translation engine #18593

atuchin-m opened this issue Oct 6, 2021 · 5 comments · Fixed by brave/brave-core#10453
Assignees
Labels
brave-translate OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/No release-notes/exclude

Comments

@atuchin-m
Copy link
Collaborator

  1. Add new base::Feature and flag (i.e. BraveInternalTranslate)
  2. Enable both buildflags enable_brave_translate_go and enable_brave_translate_extension
  3. Rewrite buidlflags-related code in the way that the new flag makes sense (not compile-time buidlflags).
  4. Make already installed translate extension to have a priority over internal translation
  5. Check that network requests are properly redirected to Brave backend (see)

for QA:

  1. check that a already installed google translate extension works well
  2. check that there is now conflicts between internal translation and extension-based
@stephendonner
Copy link
Collaborator

@atuchin-m 👋 ! Would you mind expounding on the QA section quite a bit? Step-by-step instructions would be fantastic; thanks!

@atuchin-m
Copy link
Collaborator Author

atuchin-m commented Oct 26, 2021

Hi @stephendonner.
The feature is complicated that's why there are no simple steps to check everything.

(most important) without the feature UseBraveTranslateGo:

  1. Check that nothing has changed (against current stable): user can see a bubble asking to install the Google Translate extension, the bubble works well, and user can translate sites using the extension. The internal translation is not available.

with the feature UseBraveTranslateGo/flag:

  1. (most important) If a user has enabled Google Translate extension, it has a priority (the internal translation is not available, the extension works well)
  2. If the extension is disabled, a user can a native translate bubble.
  3. The number of languages from/to translate must be equal to this set.
  4. When the backend is ready, the bubble should work: you can actually translate the page. Right now, you can test the feature only on staging with the google translate endpoint and some switches to disable security checks. After the backend will ready we need to test this scenario again.
    Command-line for staging:
    "C:\Program Files\BraveSoftware\Brave-Browser-Nightly\Application\brave.exe" --enable-features=UseBraveTranslateGo --use-google-translate-endpoint --translate-security-origin=https://translate.bravesoftware.com/ --translate-script-url=https://translate.bravesoftware.com/static/element.js --disable-web-security --disable-site-isolation-trials --user-data-dir=<some_test_temp_dir> (please replace <some_test_temp_dir> to something)

About the specs:

  1. I don't know if we have a spec for the old bubble that suggests extension installation.
  2. For the internal translation we have only a mini spec from @moritzhaller which compares several solutions. But the feature reused chromium engine/logic.
    @rebron could you help us?

Note:

  1. the internal translate engine takes into account this setting (see a screenshot). If you disabled it previously then the engine should be disabled.
  2. If you pass --translate-security-origin=/--translate-script-url= the warning bubble is ok. Otherwise, you shouldn't see it.

P.S.: also you can ask questions in slack #machine-translation
image
image

@LaurenWags
Copy link
Member

QA/Blocked pending more information on internal translations, etc. cc @rebron @brave/legacy_qa

@stephendonner
Copy link
Collaborator

stephendonner commented Dec 1, 2021

Spot-checked on both

Brave 1.33.95 Chromium: 96.0.4664.45 (Official Build) dev (64-bit)
Revision 76e4c1bb2ab4671b8beba3444e61c0f17584b2fc-refs/branch-heads/4664@{#947}
OS Linux

and

Brave 1.33.95 Chromium: 96.0.4664.45 (Official Build) dev (x86_64)
Revision 76e4c1bb2ab4671b8beba3444e61c0f17584b2fc-refs/branch-heads/4664@{#947}
OS macOS Version 11.6.1 (Build 20G224)

Confirmed:

  • Install Google Translate... dialog appears on various non-English websites (and doesn't when not needed)
  • Install button installs Google Translate
  • Don't ask again persists across tabs
  • Cancel in the dialog cancels without applying changes
  • Translate this page works
  • am not prompted by our internal engine, which remains behind the feature flag

@LaurenWags
Copy link
Member

awesome, thanks @stephendonner 👍🏻

you good with the checks from #18593 (comment) @rebron? I think they cover what we discussed 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
brave-translate OS/Desktop priority/P3 The next thing for us to work on. It'll ride the trains. QA/No release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants