[Mac/Android] Translate infobar not dismissed on client redirects |
|||||
Issue descriptionChrome Version: 64.0.3260.0 OS: Mac What steps will reproduce the problem? (1) Edit chrome/test/data/translate/update_location.html so that its script redirects to the Chrome Web Store instead of update_location_next.html. For example: location.href = "https://chrome.google.com/webstore"; (2) Visit that page in the browser (using English as the default language). What is the expected result? The translate infobar goes away after the redirect, since there's no French on the destination page. What happens instead? The translate infobar is still present on the new page, which does not need to be translated. This is due to InfoBarDelegate::ShouldExpire, which says that the infobar should not expire if details.did_replace_entry is true. It currently affects all cross-process client redirects and location.replace calls. After https://chromium-review.googlesource.com/c/chromium/src/+/567683 lands (which fixes a bug in did_replace_entry), this will affect *all* client redirects and location.replace calls, even same-process ones. We should either update the translate infobar code to dismiss on any cross-document navigation (include those with details.did_replace_entry), or convert it from infobars to bubbles (like Aura), which don't seem to share this bug. Note that this seems related to issue 236781. In particular, r201645 introduced TranslateBrowserTest.UpdateLocation which caught this issue. It's also related to r330817 and r337187, which added the did_replace_entry clause to InfoBarDelegate::ShouldExpire. I think that probably makes sense for cases like the session crashed infobar (which should not dismiss early for client redirects), but not for translate.
,
Nov 7 2017
How about mobiles? IIRC, mobiles also still use InfoBarDelegate backed UI though they have different frontends.
,
Nov 8 2017
Good question! Interestingly, it looks like Android already has this bug for both cross-process and same-process client redirects, before my CL. Tested on M62 Stable and M64 Dev. (You can test by visiting a page that has the translate infobar and typing javascript:location.replace("http://csreis.github.io") into the address bar. The infobar is not dismissed.)
I'm not sure if that's due to the same infobar code or not, given that it already happens on the same-process case.
,
Nov 8 2017
Thank you for checking it. Since this has been Android default behavior until now, this should not be a high-priority issue, but just a thing good to have.
,
Nov 13 2017
,
Nov 13 2017
Jon, I don't know who should take this bug at the moment. I've changed priority and set it to "Available".
,
Mar 7 2018
,
Mar 7 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by creis@chromium.org
, Nov 6 2017