Incorrect translation offer appears after translating a page. |
||||||||
Issue descriptionChrome Version: 64.0.3256.0 OS: iOS What steps will reproduce the problem? (1) Use an English device and navigate to "http://www.yahoo.com.tw". (2) Observe a translate offer from "Chinese (Traditional)" to "English". (3) Select to translate from "Chinese (Traditional)" to "German". (4) Perform translation and dismiss post-translate bar. This is important. (5) Scroll down the page. (5) Observe a translate offer appears for "German" to "English", even though we've already translated this Chinese website to English. What is the expected result? No translate offer should appear after we've already translated. What happens instead? A new translate offer appears with the source language as the previously selected target language.
,
Nov 1 2017
,
Nov 1 2017
+danyao in case this might be changed by her navigation work
,
Nov 1 2017
,
Nov 2 2017
,
Nov 2 2017
jzw@'s hypothesis is plausible. iOS LanguageDetectionController implements WebStateObserver::DidFinishNavigation() to detect when to kick off language detection. replaceState is currently considered a navigation by CRWWebController, so it trigger a DidFinishNavigation event. https://cs.chromium.org/chromium/src/components/translate/ios/browser/language_detection_controller.mm?q=DidFinishNavigation Line 168 is where language detection is started again if it's a same-document navigation (which replaceState is). This seems curious to me because if it's same page, the content hasn't changed, right? Why do we need to do language detection again? Perhaps someone more familiar with translate implementation can elucidate the intent. This doesn't directly affect the new navigation work because the current goal is to match current navigation stack on what events are triggered. I'd like to discuss whether replaceState and pushState should be considered "navigations", but it's probably orthogonal to this bug.
,
Nov 2 2017
replaceState and pushState are "navigations" in //content. But I'm also curious why same-document navigations cause language detection.
,
Nov 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8b55433a11a64884d40ddc8947fd93045a7aa457 commit 8b55433a11a64884d40ddc8947fd93045a7aa457 Author: John Z Wu <jzw@chromium.org> Date: Tue Nov 14 19:18:53 2017 Use new WebStateObserver method in IOSTranslateDriver. ReplaceState is treated as a navigation, but the old method "navigationItemCommited" was not called in that case and would mess up translate_manager's language_state, causing a incorrect translation offer. Bug: 780578 Change-Id: Ie4a5f55c7020a4d76168fcb8e2388a7d47fc815f Reviewed-on: https://chromium-review.googlesource.com/767043 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Commit-Queue: John Wu <jzw@chromium.org> Cr-Commit-Position: refs/heads/master@{#516380} [modify] https://crrev.com/8b55433a11a64884d40ddc8947fd93045a7aa457/components/translate/ios/browser/ios_translate_driver.h [modify] https://crrev.com/8b55433a11a64884d40ddc8947fd93045a7aa457/components/translate/ios/browser/ios_translate_driver.mm
,
Jan 18 2018
,
Jun 11 2018
,
Oct 29
jzw@, can you please confirm that this was fixed in c8, and reassign to me if it wasn't? Thanks a ton! |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by jzw@chromium.org
, Nov 1 2017