New issue
Advanced search Search tips

Issue 875035 link

Starred by 4 users
Status: Assigned
Owner:
frechette@chromium.org
Cc:
susan.boorgula@chromium.org
anthonyvd@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

lang attribute is not authoritative over CLD3 language.

Project Member Reported by frechette@chromium.org, Aug 16 
Chrome Version: 68.0.3440.106 (Official Build) (64-bit)
OS: gLinux

What steps will reproduce the problem?
(1) Go to a site such that 
  - its content language gets misidentified by CLD3.
  - it has an HTML lang tag for the right language.
Example: https://moz.com/top500; html lang attribute is `en-US`, but CLD3 thinks the content is in `pt`.

What is the expected result?
No translation offered.

What happens instead?
Translation to Portuguese is offered.

Comment 2 Deleted

Comment 3 by frechette@chromium.org, Aug 17 

Culprit seems to be https://cs.chromium.org/chromium/src/components/translate/core/language_detection/language_detection_util.cc?dr=C&g=0&l=333, specifically https://cs.chromium.org/chromium/src/components/translate/core/language_detection/language_detection_util.cc?dr=C&g=0&l=325 where, if the `html_lang` starts with `en` (i.e. is english), we check if the detected CLD language falls in a pre-determined, hard-coded list of languages that are usual culprits of having "[...] wrong server configuration of Content-Language: en":

{
    "es",    "pt", "ja", "ru", "de", "zh-CN",
    "zh-TW", "ar", "id", "fr", "it", "th"
}

So we will never trust `html_lang` whenever CLD thinks the content is in this language.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 26 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/530abd79f10af8ec610559457e5dd2d746d74aa1

commit 530abd79f10af8ec610559457e5dd2d746d74aa1
Author: Alexandre Frechette <frechette@chromium.org>
Date: Wed Sep 26 15:30:11 2018

[Language] Do not show translation offer prompt when CLD and html/content
languages differ.

There are cases where CLD is wrong, it differs from the HTML or content
languages (signals from the webpage) and we still trust it. In such
situation, we should not actively suggest translation, and simply show
the icon.

We achieve this by passing the signals that were used during
language detection to the component in charge of showing translation.

Bug: 875035
Change-Id: Icbaf6ec19aeffeb246c7af03c18ee75d97fa7eeb
Reviewed-on: https://chromium-review.googlesource.com/1221687
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Michael Martis <martis@chromium.org>
Reviewed-by: anthonyvd <anthonyvd@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alexandre Frechette <frechette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594323}
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/chrome/browser/android/contextualsearch/contextual_search_context.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/content/browser/content_translate_driver.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/content/common/translate.mojom
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/content/common/translate_struct_traits.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/content/common/translate_struct_traits.h
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/content/renderer/translate_helper.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/browser/language_state.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/browser/language_state.h
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/browser/language_state_unittest.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/browser/translate_browser_metrics.h
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/browser/translate_manager.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/browser/translate_manager_unittest.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/common/language_detection_details.h
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/language_detection/language_detection_util.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/language_detection/language_detection_util.h
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/core/language_detection/language_detection_util_unittest.cc
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/ios/browser/ios_translate_driver.mm
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/components/translate/ios/browser/language_detection_controller.mm
[modify] https://crrev.com/530abd79f10af8ec610559457e5dd2d746d74aa1/testing/libfuzzer/fuzzers/language_detection_fuzzer.cc

Comment 5 by frechette@chromium.org, Sep 27

Status: Fixed (was: Started)
The agreed upon mitigation for this bug was to avoid showing translation for conflicting cases. The above CL achieves that.

Comment 6 by frechette@chromium.org, Oct 1

Status: Started (was: Fixed)

Comment 7 by frechette@chromium.org, Oct 1 

Potentially problematic page language scenarios:
- If HTML lang and content lang are different (HTML lang is typically authoritative between the two).
- Will suppress translation when CLD complements with a subcode. https://cs.chromium.org/chromium/src/components/translate/core/language_detection/language_detection_util.cc?rcl=5e3bf2a1f618d292d253ee5b0236fcd2cb909c50&l=213
- If the "server configuration is wrong" (https://cs.chromium.org/chromium/src/components/translate/core/language_detection/language_detection_util.cc?rcl=5e3bf2a1f618d292d253ee5b0236fcd2cb909c50&l=225) we will suppress translation.
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 7 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89d6f0d7ea7982e72db195523356338171977c49

commit 89d6f0d7ea7982e72db195523356338171977c49
Author: Alexandre Frechette <frechette@chromium.org>
Date: Sun Oct 07 21:05:33 2018

Revert "[Language] Do not show translation offer prompt when CLD and html/content"

This reverts commit 530abd79f10af8ec610559457e5dd2d746d74aa1.

Reason for revert:
This CL needs to be reworked. It causes a memory regression (https://bugs.chromium.org/p/chromium/issues/detail?id=891257), and also has badly addressed edge cases (see thread in https://bugs.chromium.org/p/chromium/issues/detail?id=875035, causing bug https://bugs.chromium.org/p/chromium/issues/detail?id=892441).

Original change's description:
> [Language] Do not show translation offer prompt when CLD and html/content
> languages differ.
>
> There are cases where CLD is wrong, it differs from the HTML or content
> languages (signals from the webpage) and we still trust it. In such
> situation, we should not actively suggest translation, and simply show
> the icon.
>
> We achieve this by passing the signals that were used during
> language detection to the component in charge of showing translation.
>
> Bug: 875035
> Change-Id: Icbaf6ec19aeffeb246c7af03c18ee75d97fa7eeb
> Reviewed-on: https://chromium-review.googlesource.com/1221687
> Reviewed-by: Jonathan Metzman <metzman@chromium.org>
> Reviewed-by: Theresa <twellington@chromium.org>
> Reviewed-by: Michael Martis <martis@chromium.org>
> Reviewed-by: anthonyvd <anthonyvd@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: Alexandre Frechette <frechette@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#594323}

TBR=dcheng@chromium.org,twellington@chromium.org,anthonyvd@chromium.org,martis@chromium.org,metzman@chromium.org,frechette@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 875035,  892441 ,  891257 ,  892783 
Change-Id: I7c839fa96bee25b2a9332b3156f1de622bb47e19
Reviewed-on: https://chromium-review.googlesource.com/c/1263893
Commit-Queue: Alexandre Frechette <frechette@chromium.org>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597467}
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/chrome/browser/android/contextualsearch/contextual_search_context.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/content/browser/content_translate_driver.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/content/common/translate.mojom
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/content/common/translate_struct_traits.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/content/common/translate_struct_traits.h
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/content/renderer/translate_helper.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/browser/language_state.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/browser/language_state.h
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/browser/language_state_unittest.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/browser/translate_browser_metrics.h
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/browser/translate_manager.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/browser/translate_manager_unittest.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/common/language_detection_details.h
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/language_detection/language_detection_util.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/language_detection/language_detection_util.h
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/core/language_detection/language_detection_util_unittest.cc
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/ios/browser/ios_translate_driver.mm
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/components/translate/ios/browser/language_detection_controller.mm
[modify] https://crrev.com/89d6f0d7ea7982e72db195523356338171977c49/testing/libfuzzer/fuzzers/language_detection_fuzzer.cc

Comment 9 by anthonyvd@chromium.org, Oct 17

Cc: susan.boorgula@chromium.org
 Issue 892938  has been merged into this issue.

Comment 10 by frechette@chromium.org, Nov 1

Status: Assigned (was: Started)
►

Sign in to add a comment