Issue metadata
Sign in to add a comment
|
Regression: Favicons of synced urls are seen missing in chrome://history after navigating back. |
||||||||||||||||||||||
Issue descriptionVersion: 53.0.2761.0 dev OS: Ubuntu 14.04, windows What steps will reproduce the problem? (1) Sign in to chrome so that history of other devices is synced >> Now click on "# more.." link of any of the synced history and open link in new tab (2) Now navigate to opened tab >> Switch back to history >> Now click on "# more" link of other synced history and observe for favicons[Please refer video] Expected: Favicons of synced history should be seen even after navigating back Actual: Instead favicons are seen missing. This is a regression issue broken in M52. Unable to do tool bisect as we can't login to chromium builds. Hence providing Manual bisect info. Good Build: 52.0.2718.0 dev Bad Build: 52.0.2719.0 dev CR: https://chromium.googlesource.com/chromium/src/+log/52.0.2718.0..52.0.2719.0?pretty=fuller&n=10000 Suspecting https://codereview.chromium.org/1914073005 from changelog. @dpapad: Please confirm the issue and help in re-assigning if it is not related to your change.
,
Jun 8 2016
I am able to reproduce the problem. It is caused by a CSP violation see console errors in the attachment. I am still not sure if it is caused by my https://codereview.chromium.org/1914073005. Will keep investigating.
,
Jun 8 2016
I was able to perform a more detailed bisection, and came up with narrower CL range (that does not include https://codereview.chromium.org/1914073005). https://chromium.googlesource.com/chromium/src/+log/5433e667fc2a2a1e62e969a8d7539081c4dde9c3..94d5fad34f0351e61fd658ca4ed93648c415620d Perhaps the V8 roll at https://chromium.googlesource.com/chromium/src/+/47aee4ab460a9f4ce895794d23c73fe02c2f4548, could be related?
,
Jun 8 2016
could this be CSP-related?
,
Jun 8 2016
The problem seems to begin as soon as middle-clicking on a link (see attached video), which makes me think that https://chromium.googlesource.com/chromium/src/+/76fea00a18f75886ea649414393228180306e13d could also be related.
,
Jun 9 2016
Verified that https://chromium.googlesource.com/chromium/src/+/76fea00a18f75886ea649414393228180306e13d is the culprit, by reverting it locally. Assigning to @nzolghadr for further investigation.
,
Jun 15 2016
Just to update: Able to reproduce the issue on win8.1 chrome version 53.0.2768.0 nzolghadr@, Could you please take a look
,
Jun 15 2016
I was looking at it to be honest with you. There are a few bugs related to Issue 255 . Although that seems weird the icon loading being related to that change but the console errors are most likely related as after that change we are relying on Chrome to open the tab and not the js as we don't send the click event for middle button anymore. I might be reverting Issue 255 fix altogether. Do you mind this issue waits until the end of June so I can gather more data on what parts Issue 255 affects?
,
Jun 16 2016
A friendly reminder that M52 Stable is launching soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch by July 12. All changes MUST be merged into the release branch by 5pm on July 15 to make into the desktop Stable final build cut. Thank you!
,
Jun 23 2016
nzolghadr@ : Could you please take a look into this and update further on it. On the latest canary 53.0.2777.0 the Chrome://history page is shown in according to material design.While tried with non-material(setting it on the chrome://flags) the chrome;//history page is still shown as of material-design only.
,
Jun 27 2016
** IMPORTANT change in M52 merge date due to first 2 weeks of July no release weeks ** M52 Stable is launching very soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged ASAP. All changes MUST be merged into the release branch by 5pm on July 1 to make into the desktop Stable final build cut. Thank you!
,
Jun 29 2016
A CL has been posted for this issue: https://codereview.chromium.org/2107093003/
,
Jul 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40 commit 4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40 Author: nzolghadr <nzolghadr@chromium.org> Date: Fri Jul 01 19:02:51 2016 Revert middle click related changes This is the CL to revert the related middle button click changes. We decided to revert those changes because the issues that were caused by suppressing click event for middle button was hard to fix without having that event. Particularly the ability to prevent opening a new tab which can be done by "preventDefault"ing the click event of middle button was removed as the result of the original change. For now we revert these changes and we pursue the line of adding a new event for non-primary button click to be able to fix these problem in a more clean way. Revert "Prevent sending click event for non primary button" This reverts commit 76fea00a18f75886ea649414393228180306e13d. Revert "Dispatch middle click manually by tracking mouse" This reverts commit 88eb1110baafcba070e750866a343e81b6bcc524. Revert "Fix history page middle click action" This reverts commit a154aed1a3813cf28c6f477579ed7974a2528570. BUG= 255 , 618593 , 617444 , 611019 , 617875 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2107093003 Cr-Commit-Position: refs/heads/master@{#403496} [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/chrome/browser/resources/history/history.js [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/chrome/browser/resources/ntp4/new_tab.html [delete] https://crrev.com/afd007a5a072039641b7b81eddd9aa488cec4996/chrome/browser/resources/synthetic_middleclick.js [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/LayoutTests/fast/events/script-tests/mouse-click-events.js [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/Source/core/events/MouseEvent.cpp [modify] https://crrev.com/4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40/third_party/WebKit/Source/core/input/EventHandler.cpp
,
Jul 1 2016
M52 Stable is launching very soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged ASAP. All changes MUST be merged into the release branch by 5pm on July 8th (in case if you missed today's 5:00 PM PST deadline) to make into the desktop Stable final build cut. Thank you!
,
Jul 4 2016
Tested this issue on Ubuntu 14.04 and Window 7 using chrome latest canary M54-54.0.2787.0 by following steps mentioned in the original comment. Observed favicons of synced urls are able to see now in chrome://history after navigating back to the previous page. Hence adding TE-Verified label.
,
Jul 4 2016
nzolghadr@, Could you please merge this to M53 since verified in M54
,
Jul 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637 commit f0dfcc7e4b98d8c891bb3a8bf03900403aab0637 Author: Dave Tapuska <dtapuska@chromium.org> Date: Mon Jul 04 14:25:31 2016 Revert middle click related changes This is the CL to revert the related middle button click changes. We decided to revert those changes because the issues that were caused by suppressing click event for middle button was hard to fix without having that event. Particularly the ability to prevent opening a new tab which can be done by "preventDefault"ing the click event of middle button was removed as the result of the original change. For now we revert these changes and we pursue the line of adding a new event for non-primary button click to be able to fix these problem in a more clean way. Revert "Prevent sending click event for non primary button" This reverts commit 76fea00a18f75886ea649414393228180306e13d. Revert "Dispatch middle click manually by tracking mouse" This reverts commit 88eb1110baafcba070e750866a343e81b6bcc524. Revert "Fix history page middle click action" This reverts commit a154aed1a3813cf28c6f477579ed7974a2528570. BUG= 255 , 618593 , 617444 , 611019 , 617875 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2107093003 Cr-Commit-Position: refs/heads/master@{#403496} (cherry picked from commit 4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40) Review URL: https://codereview.chromium.org/2121003002 . Cr-Commit-Position: refs/branch-heads/2785@{#12} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/chrome/browser/resources/history/history.js [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/chrome/browser/resources/ntp4/new_tab.html [delete] https://crrev.com/aaad16cd6a623d566e75f072db80264e4a5836ac/chrome/browser/resources/synthetic_middleclick.js [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/LayoutTests/fast/events/script-tests/mouse-click-events.js [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/Source/core/events/MouseEvent.cpp [modify] https://crrev.com/f0dfcc7e4b98d8c891bb3a8bf03900403aab0637/third_party/WebKit/Source/core/input/EventHandler.cpp
,
Jul 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e commit eaaefc24ebb90ea1586f1ce7aee411da3da9d16e Author: Dave Tapuska <dtapuska@chromium.org> Date: Mon Jul 04 14:43:01 2016 Revert middle click related changes This is the CL to revert the related middle button click changes. We decided to revert those changes because the issues that were caused by suppressing click event for middle button was hard to fix without having that event. Particularly the ability to prevent opening a new tab which can be done by "preventDefault"ing the click event of middle button was removed as the result of the original change. For now we revert these changes and we pursue the line of adding a new event for non-primary button click to be able to fix these problem in a more clean way. Revert "Prevent sending click event for non primary button" This reverts commit 76fea00a18f75886ea649414393228180306e13d. Revert "Dispatch middle click manually by tracking mouse" This reverts commit 88eb1110baafcba070e750866a343e81b6bcc524. Revert "Fix history page middle click action" This reverts commit a154aed1a3813cf28c6f477579ed7974a2528570. BUG= 255 , 618593 , 617444 , 611019 , 617875 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2107093003 Cr-Commit-Position: refs/heads/master@{#403496} (cherry picked from commit 4d9d9183fe8c1a4dbfac9f4a6ef8a337b05c1a40) Review URL: https://codereview.chromium.org/2124533002 . Cr-Commit-Position: refs/branch-heads/2743@{#580} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/chrome/browser/resources/history/history.js [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/chrome/browser/resources/ntp4/new_tab.html [delete] https://crrev.com/5f87571138ba5e85759984e956efcb6691cf3770/chrome/browser/resources/synthetic_middleclick.js [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/chrome/browser/ui/browser_browsertest.cc [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/testing/buildbot/filters/browser-side-navigation.linux.browser_tests.filter [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/LayoutTests/fast/events/mouse-click-events-expected.txt [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/LayoutTests/fast/events/script-tests/mouse-click-events.js [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/Source/core/events/MouseEvent.cpp [modify] https://crrev.com/eaaefc24ebb90ea1586f1ce7aee411da3da9d16e/third_party/WebKit/Source/core/input/EventHandler.cpp
,
Jul 5 2016
brajkumar@ if you are testing this can you also test this on M52 as well as M53?
,
Jul 6 2016
nzolghadr@, we will get this verified for the next upcoming Dev & Beta releases scheduled.
,
Jul 8 2016
Added the respective labels as per the above comment #22.
,
Jul 13 2016
Rechecked this on Windows 7, Ubuntu 14.04 for chrome version 52.0.2743.75 and merge is working as intended. Favicons of synced urls are displayed in chrome://history after a re navigation. Adding TE-Verified labels.
,
Sep 27 2016
,
Oct 5 2016
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ssamanoori@chromium.org
, Jun 7 2016