Issue metadata
Sign in to add a comment
|
Regression: Translate bubble doesn’t appear for https://www.tmall.com/.
Reported by
dchau...@etouch.net,
Mar 27 2018
|
||||||||||||||||||||||||
Issue descriptionChrome Version: 66.0.3359.63 (Official Build) Revision 0dae3cfa5df6cc48cce0edbded58c9b8e62b1fa8-refs/branch-heads/3359@{#458} 32/64-bit. OS: Win(7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.13.4) and Linux (14.04 LTS). Test-URL:- https://www.tmall.com/ What steps will reproduce the problem? 1. Launch Chrome, navigate to above URL and observe. Actual: Translate bubble doesn’t appear. Expected: Translate bubble should appear. This is a regression issue, broken in M-66 series, below is manual regression range. Good build: 66.0.3348.0 (Revision: 536935) Bad build: 66.0.3349.0 (Revision: 537110) Using the per-revision bisect providing the bisect results: You are probably looking for a change made after 536968 (known good), but no later than 536969 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/a85265e3f9b269648fab357695479fd56fcd1e2d..605ee9330b375a6e91a99606be0800e6331fe524 Suspecting: https://chromium.googlesource.com/chromium/src/+/605ee9330b375a6e91a99606be0800e6331fe524 @xiaochengh: Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner. NOTE: This issue is also reproducible on M-66 Beta (build # 66.0.3359.45), M-67 Dev (build # 67.0.3377.1) and M-67 Canary (build # 67.0.3379.0) Kindly review the attached screen-cast for reference. Thank you.
,
Mar 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/802df0571195f064c2bd77b513b86950eeb9f52a commit 802df0571195f064c2bd77b513b86950eeb9f52a Author: Xiaocheng Hu <xiaochengh@chromium.org> Date: Sat Mar 31 00:26:30 2018 Make CapurePageText capture textContent only on dirty layout This is a partial revert for r536969 Change TranslateHelper to use a TextContent-like text dump. r536969 is a workaround for a lifecycle violation bug that layout is still dirty after running layout update. It turns out that textContent is not a good text dump for TranslateHelper, as pages may hide arbitrary content (e.g., JSON) in invisible text nodes, which can't be reliably filtered without checking element style or layout. Since we don't always run CapturePageText() with dirty layout, there is no need to always dump textContent. Hence, this patch partially reverts r536969 that, we still dump innerText as long as layout is clean. We dump textContent only when layout is dirty, which is enough to get around the lifecycle violation bug. Bug: 826174 Change-Id: I63e97832caa9858e67992847fb9a22f6405183cb Reviewed-on: https://chromium-review.googlesource.com/984474 Reviewed-by: Kent Tamura <tkent@chromium.org> Reviewed-by: Rachel Blum <groby@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#547334} [modify] https://crrev.com/802df0571195f064c2bd77b513b86950eeb9f52a/third_party/WebKit/Source/core/exported/WebFrameContentDumper.cpp
,
Apr 1 2018
Should have been fixed in Canary 67.0.3385.0 and later version.
,
Apr 2 2018
With response to comment #3: Re-tested this issue on Win(7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.13.4) and Linux (14.04 LTS) machines using latest Canary build # 67.0.3386.0 (Official Build). Issue is fixed on Mac(10.12.6, 10.13.1, 10.13.4) OS and working as intended i.e. translate bubble auto pop-up appears on navigating to https://www.tmall.com/ But issue is not fixed on Win(7,8,8.1,10) and Linux(14.04 LTS) machines i.e. Translate icon appears in omnibox but translate bubble auto pop-up doesn't appear. Attaching screen-cast for the same.
,
Apr 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f397777d8c4e6855f580f0f2d1a1ddd9b0eec217 commit f397777d8c4e6855f580f0f2d1a1ddd9b0eec217 Author: Xiaocheng Hu <xiaochengh@chromium.org> Date: Tue Apr 03 19:53:06 2018 Revert "Make CapurePageText capture textContent only on dirty layout" This reverts commit 802df0571195f064c2bd77b513b86950eeb9f52a. Reason for revert: suspected for causing memory regressions crbug.com/828026 and crbug.com/828027 Original change's description: > Make CapurePageText capture textContent only on dirty layout > > This is a partial revert for r536969 Change TranslateHelper to use a > TextContent-like text dump. > > r536969 is a workaround for a lifecycle violation bug that layout is > still dirty after running layout update. > > It turns out that textContent is not a good text dump for > TranslateHelper, as pages may hide arbitrary content (e.g., JSON) in > invisible text nodes, which can't be reliably filtered without checking > element style or layout. > > Since we don't always run CapturePageText() with dirty layout, there is > no need to always dump textContent. Hence, this patch partially reverts > r536969 that, we still dump innerText as long as layout is clean. We > dump textContent only when layout is dirty, which is enough to get > around the lifecycle violation bug. > > Bug: 826174 > Change-Id: I63e97832caa9858e67992847fb9a22f6405183cb > Reviewed-on: https://chromium-review.googlesource.com/984474 > Reviewed-by: Kent Tamura <tkent@chromium.org> > Reviewed-by: Rachel Blum <groby@chromium.org> > Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#547334} TBR=groby@chromium.org,tkent@chromium.org,xiaochengh@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 826174 , 828026, 828027 Change-Id: I4c3200e5c7de0e6b3838ddcc40d53e751538f915 Reviewed-on: https://chromium-review.googlesource.com/992818 Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Cr-Commit-Position: refs/heads/master@{#547796} [modify] https://crrev.com/f397777d8c4e6855f580f0f2d1a1ddd9b0eec217/third_party/WebKit/Source/core/exported/WebFrameContentDumper.cpp
,
Apr 4 2018
Update:- Re-tested this issue on Windows (7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.13.4) and Linux(14.04 LTS) machines using latest Chrome Canary build# 67.0.3388.0 and issue is still persist. Attaching screen-cast for the same.
,
Apr 5 2018
All relevant changes have been reverted: 98199e90 Revert "Change TranslateHelper to use a textContent-like text dump" by Xiaocheng Hu · 23 hours ago ccfb59a Revert "Eliminate NOSCRIPT element from TranslateHelper text dump" by Xiaocheng Hu · 27 hours ago f397777 Revert "Make CapurePageText capture textContent only on dirty layout" by Xiaocheng Hu · 33 hours ago 802df05 Make CapurePageText capture textContent only on dirty layout by Xiaocheng Hu · 5 days ago 797a380 Eliminate NOSCRIPT element from TranslateHelper text dump by Xiaocheng Hu · 7 weeks ago 605ee93 Change TranslateHelper to use a textContent-like text dump by Xiaocheng Hu · 7 weeks ago The first version with all reverts is 67.0.3389.0. Test team: could you re-test if there is still any issue with 67.0.3389.0 or later version, and re-triage if it still reproduces? Thanks!
,
Apr 5 2018
With response to comment #7: Re-tested this issue on Win(7,8,8.1,10), Mac(10.12.6, 10.13.1, 10.13.4) and Linux (14.04 LTS) machines using latest Canary build # 67.0.3389.0 (Official Build). Issue is fixed in Mac(10.12.6, 10.13.1, 10.13.4) OS and working as intended i.e. translate bubble auto pop-up appears on navigating to https://www.tmall.com/ But issue is not fixed on Win(7,8,8.1,10) and Linux(14.04 LTS) machines i.e. Translate icon appears in omnibox but translate bubble auto pop-up doesn't appear. NOTE: In Win(7,8,8.1,10) and Linux(14.04 LTS) machines, some time translate bubble auto pop-up appears, but it is very much inconsistent. Attaching screen-cast for the same.
,
Apr 5 2018
As all my changes have been reverted, the "no translate pop-up" issue should be due to some other reason. groby@: Could you help triaging? Thanks!
,
May 20 2018
|
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by xiaoche...@chromium.org
, Mar 28 2018