Issue metadata
Sign in to add a comment
|
MostVisited icon title is incorrectly updated |
||||||||||||||||||||||
Issue descriptionApp Version: M57.0.2987.20 beta iOS Version: 10.3, 9.3.5 Device: iPhone7, iPad mini URL: na Steps to reproduce: 1. Launch Google Chrome Beta 2. (Make sure you don't have any tiles in NTP, Clear browsing data if required) 3. Visit apple.com 4. Open a new new tab and make sure one tile is updated for Apple 5. Goto back to previous tab, and navigate to bbc.com 6. Once the page loaded, tap on back arrow twice to reach NTP Observed results: Apple tile title is displayed as bbc.com, also a new tile created for bbc.com Expected results: Apple tile title shouldn't be updated. Number of times you were able to reproduce: 5/5 Bug reproducible after clean install: Yes Bug reproducible after clearing cache and cookies: Yes Bug reproducible on Chrome Mobile on Android: NA Bug reproducible on Safari/Firefox: Firefox: NA, Safari: NA Bug reproducible on current stable build (App Version, iOS Version): M56 NO Bug reproducible on the current beta channel build (App Version, iOS Version): M57 Yes Link to video/image: https://drive.google.com/file/d/0B-xmXLQhjeKucXRvTWFlclNVZXM/view
,
Feb 2 2017
This seems a really bad regression! I had a quick attempt with Clank dev (M57.0.2987.19) and can't repro, although it seems unlikely that it's iOS-specific. How does History show up in this scenario? Is the mismatch visible there too?
,
Feb 2 2017
Yes. History is also effected https://drive.google.com/file/d/0B-xmXLQhjeKuNVhPcHN4amhVZmc/view
,
Feb 3 2017
I was able to repro on iOS near HEAD (built Wed at 522533e188). Assuming apple.com for the first site visited and bbc.com for the second, then what I get is Tile 1 URL http://www.apple.com/ (correct) Title www.bbc.com (wrong) Icon None (??? [1]) Tile 2 URL http://www.bbc.com/ (correct) Title BBC (correct) Icon BBC icon (correct) I also tried this with the fully-qualified URLS http://www.apple.com/ and http://www.bbc.com/ (to verify that this is not a redirect bug). This bug still happens in this case, so it's different from crbug/670271 (but may still be related). I also tried with the following: 1. In tab #1, go to www.apple.com, then www.bbc.com 2. Open tab #2, go to www.chromium.org 3. Back in tab #1, go back 4. Go to new tab This still reproduces, with the problem still observable in history. The fact that this shows up in history, and doesn't seem to be reproducible on Android, suggests to me that the bug resides in Chrome iOS's history implementation, which TopSites then passes on to the NTP (I was able to repro with popular sites enabled—the bug seems to be limited to TopSites specifically) [1] Weirdly, I tried both apple.com and amazon.com for tile #1 but didn't get apple-touch-icons. I *know* both of those sites have them. History shows an icon for apple.com but I think it's the favicon; hard to tell, but I think it's lower-resolution than the BBC and Chromium icons shown next to it in history.
,
Feb 3 2017
I see a few patches by kkhorimoto@ which could be related. Perhaps try reverting be48752128d7ec4a5bd6d980157deb60dbb29548? I myself don't have a working setup for iOS development. More likely, we'll need to bisect.
,
Feb 3 2017
Based on your observations in comment#4, this seems tobe a problem with PendingIndex navigation. Issue reproduces with Pending Index Navigation is enabled and doesn't repro when disabling PendingIndex navigation. cc'ing eugenebut@ owner of this feature.
,
Feb 3 2017
,
Feb 3 2017
,
Feb 6 2017
,
Feb 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9fc98ecff33175b1bc234f9c6c3f42c07285dc1f commit 9fc98ecff33175b1bc234f9c6c3f42c07285dc1f Author: eugenebut <eugenebut@chromium.org> Date: Tue Feb 07 22:53:33 2017 Extended HistoryUITestCase to test titles. Also added disabled testHistoryUpdateAfterBackNavigation as a test case for crbug.com/688047 and cleaned up WebViewContainingText matcher usage. BUG= 688047 Review-Url: https://codereview.chromium.org/2683453002 Cr-Commit-Position: refs/heads/master@{#448762} [modify] https://crrev.com/9fc98ecff33175b1bc234f9c6c3f42c07285dc1f/ios/chrome/browser/ui/history/history_ui_egtest.mm
,
Feb 7 2017
Fix on review: https://codereview.chromium.org/2685803002/
,
Feb 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69378e0417fa4dbcae4dc863e5b46b68d71ac0be commit 69378e0417fa4dbcae4dc863e5b46b68d71ac0be Author: stkhapugin <stkhapugin@chromium.org> Date: Fri Feb 10 12:10:09 2017 Revert of Fixed title updating for back forward navigation. (patchset #3 id:40001 of https://codereview.chromium.org/2685803002/ ) Reason for revert: Breaks -[TabHistoryPopupControllerTestCase testTabHistoryMenu] tab_history_popup_controller_egtest.mm downstream, because New Tab in history popup (when long pressed on back) displays "chrome://newtab" instead of "New Tab". I actually don't know why this test is not ran upstream, but I have to revert, sorry! Original issue's description: > Fixed title updating for back forward navigation. > > Chrome relies on KVO compliant "title" property to subscribe for > title updates. If this KVO change happens during then navigation > it is unclear if title was changes for the previous page, or the > navigation was committed and title was changed for the new page. > > So if there is a navigation in progress WebController should ignore > KVO change, but it should always attempt to update title when > navigation is committed. > > BUG= 688047 , 677356 > > Review-Url: https://codereview.chromium.org/2685803002 > Cr-Commit-Position: refs/heads/master@{#449527} > Committed: https://chromium.googlesource.com/chromium/src/+/487e4cfd53bac5944dad49cbce3b939a0c7bf035 TBR=kkhorimoto@chromium.org,eugenebut@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 688047 , 677356 Review-Url: https://codereview.chromium.org/2684343003 Cr-Commit-Position: refs/heads/master@{#449595} [modify] https://crrev.com/69378e0417fa4dbcae4dc863e5b46b68d71ac0be/ios/chrome/browser/tabs/tab.mm [modify] https://crrev.com/69378e0417fa4dbcae4dc863e5b46b68d71ac0be/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/69378e0417fa4dbcae4dc863e5b46b68d71ac0be/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/69378e0417fa4dbcae4dc863e5b46b68d71ac0be/ios/web/web_state/ui/crw_web_controller_unittest.mm [modify] https://crrev.com/69378e0417fa4dbcae4dc863e5b46b68d71ac0be/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/69378e0417fa4dbcae4dc863e5b46b68d71ac0be/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/69378e0417fa4dbcae4dc863e5b46b68d71ac0be/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
Feb 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/adb03a4e58c33d0adc812a2ddae2180abe8ac148 commit adb03a4e58c33d0adc812a2ddae2180abe8ac148 Author: eugenebut <eugenebut@chromium.org> Date: Sat Feb 11 02:03:52 2017 Relanding "Fixed title updating for back forward navigation." Chrome relies on KVO compliant "title" property to subscribe for title updates. If this KVO change happens during then navigation it is unclear if title was changes for the previous page, or the navigation was committed and title was changed for the new page. So if there is a navigation in progress WebController should ignore KVO change, but it should always attempt to update title when navigation is committed. BUG= 688047 , 677356 Review-Url: https://codereview.chromium.org/2685803002 Cr-Commit-Position: refs/heads/master@{#449527} (cherry picked from commit 487e4cfd53bac5944dad49cbce3b939a0c7bf035) Review-Url: https://codereview.chromium.org/2691693002 Cr-Commit-Position: refs/heads/master@{#449828} [modify] https://crrev.com/adb03a4e58c33d0adc812a2ddae2180abe8ac148/ios/chrome/browser/tabs/tab.mm [modify] https://crrev.com/adb03a4e58c33d0adc812a2ddae2180abe8ac148/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/adb03a4e58c33d0adc812a2ddae2180abe8ac148/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/adb03a4e58c33d0adc812a2ddae2180abe8ac148/ios/web/web_state/ui/crw_web_controller_unittest.mm [modify] https://crrev.com/adb03a4e58c33d0adc812a2ddae2180abe8ac148/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/adb03a4e58c33d0adc812a2ddae2180abe8ac148/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/adb03a4e58c33d0adc812a2ddae2180abe8ac148/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
Feb 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d commit 6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d Author: eugenebut <eugenebut@chromium.org> Date: Sun Feb 12 02:04:08 2017 Revert of Relanding "Fixed title updating for back forward navigation." (patchset #3 id:40001 of https://codereview.chromium.org/2691693002/ ) Reason for revert: Breaks some other tests this time: https://uberchromegw.corp.google.com/i/internal.bling.main/builders/iphone9-simulator-x64/builds/9939 Original issue's description: > Relanding "Fixed title updating for back forward navigation." > > Chrome relies on KVO compliant "title" property to subscribe for > title updates. If this KVO change happens during then navigation > it is unclear if title was changes for the previous page, or the > navigation was committed and title was changed for the new page. > > So if there is a navigation in progress WebController should ignore > KVO change, but it should always attempt to update title when > navigation is committed. > > BUG= 688047 , 677356 > > Review-Url: https://codereview.chromium.org/2685803002 > Cr-Commit-Position: refs/heads/master@{#449527} > (cherry picked from commit 487e4cfd53bac5944dad49cbce3b939a0c7bf035) > > Review-Url: https://codereview.chromium.org/2691693002 > Cr-Commit-Position: refs/heads/master@{#449828} > Committed: https://chromium.googlesource.com/chromium/src/+/adb03a4e58c33d0adc812a2ddae2180abe8ac148 TBR=kkhorimoto@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 688047 , 677356 Review-Url: https://codereview.chromium.org/2689083002 Cr-Commit-Position: refs/heads/master@{#449881} [modify] https://crrev.com/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d/ios/chrome/browser/tabs/tab.mm [modify] https://crrev.com/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d/ios/web/web_state/ui/crw_web_controller_unittest.mm [modify] https://crrev.com/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
Feb 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/176138f21cb2df89eb6157bb748b60207038eff4 commit 176138f21cb2df89eb6157bb748b60207038eff4 Author: eugenebut <eugenebut@chromium.org> Date: Sun Feb 12 04:14:48 2017 Reland of landing "Fixed title updating for back forward navigation." (patchset #1 id:1 of https://codereview.chromium.org/2689083002/ ) Reason for revert: This revert did not help. Reverting the revert. Original issue's description: > Revert of Relanding "Fixed title updating for back forward navigation." (patchset #3 id:40001 of https://codereview.chromium.org/2691693002/ ) > > Reason for revert: > Breaks some other tests this time: > https://uberchromegw.corp.google.com/i/internal.bling.main/builders/iphone9-simulator-x64/builds/9939 > > Original issue's description: > > Relanding "Fixed title updating for back forward navigation." > > > > Chrome relies on KVO compliant "title" property to subscribe for > > title updates. If this KVO change happens during then navigation > > it is unclear if title was changes for the previous page, or the > > navigation was committed and title was changed for the new page. > > > > So if there is a navigation in progress WebController should ignore > > KVO change, but it should always attempt to update title when > > navigation is committed. > > > > BUG= 688047 , 677356 > > > > Review-Url: https://codereview.chromium.org/2685803002 > > Cr-Commit-Position: refs/heads/master@{#449527} > > (cherry picked from commit 487e4cfd53bac5944dad49cbce3b939a0c7bf035) > > > > Review-Url: https://codereview.chromium.org/2691693002 > > Cr-Commit-Position: refs/heads/master@{#449828} > > Committed: https://chromium.googlesource.com/chromium/src/+/adb03a4e58c33d0adc812a2ddae2180abe8ac148 > > TBR=kkhorimoto@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 688047 , 677356 > > Review-Url: https://codereview.chromium.org/2689083002 > Cr-Commit-Position: refs/heads/master@{#449881} > Committed: https://chromium.googlesource.com/chromium/src/+/6cc15b2c2eb82ac455cd7dbfe912be9914c1c03d TBR=kkhorimoto@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 688047 , 677356 Review-Url: https://codereview.chromium.org/2694583002 Cr-Commit-Position: refs/heads/master@{#449886} [modify] https://crrev.com/176138f21cb2df89eb6157bb748b60207038eff4/ios/chrome/browser/tabs/tab.mm [modify] https://crrev.com/176138f21cb2df89eb6157bb748b60207038eff4/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/176138f21cb2df89eb6157bb748b60207038eff4/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/176138f21cb2df89eb6157bb748b60207038eff4/ios/web/web_state/ui/crw_web_controller_unittest.mm [modify] https://crrev.com/176138f21cb2df89eb6157bb748b60207038eff4/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/176138f21cb2df89eb6157bb748b60207038eff4/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/176138f21cb2df89eb6157bb748b60207038eff4/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
Feb 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e0da344ce088f22ac29af6084d5fb7b83f3624f commit 3e0da344ce088f22ac29af6084d5fb7b83f3624f Author: eugenebut <eugenebut@chromium.org> Date: Tue Feb 14 01:30:51 2017 Enabled testHistoryUpdateAfterBackNavigation. BUG= 688047 Review-Url: https://codereview.chromium.org/2687233002 Cr-Commit-Position: refs/heads/master@{#450189} [modify] https://crrev.com/3e0da344ce088f22ac29af6084d5fb7b83f3624f/ios/chrome/browser/ui/history/history_ui_egtest.mm
,
Feb 14 2017
,
Feb 14 2017
This bug requires manual review: Less than 2 weeks to go before AppStore submit on M57 Please contact the milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 14 2017
Verified on chrome canary version 58.0.3012.0 on iPad4 10.2.1,iPhone 7plus 10.2.1, following the comments mentioned in comment 0 observed that the most visited icon title is displayed correctly.
,
Feb 14 2017
,
Feb 15 2017
,
Feb 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55217cf9efff8866160932c1614077d4ed43a2d3 commit 55217cf9efff8866160932c1614077d4ed43a2d3 Author: Eugene But <eugenebut@google.com> Date: Wed Feb 15 16:30:32 2017 Fixed title updating for back forward navigation. Chrome relies on KVO compliant "title" property to subscribe for title updates. If this KVO change happens during then navigation it is unclear if title was changes for the previous page, or the navigation was committed and title was changed for the new page. So if there is a navigation in progress WebController should ignore KVO change, but it should always attempt to update title when navigation is committed. BUG= 688047 , 677356 Review-Url: https://codereview.chromium.org/2685803002 Cr-Commit-Position: refs/heads/master@{#449527} (cherry picked from commit 487e4cfd53bac5944dad49cbce3b939a0c7bf035) Review-Url: https://codereview.chromium.org/2701463002 . Cr-Commit-Position: refs/branch-heads/2987@{#521} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/55217cf9efff8866160932c1614077d4ed43a2d3/ios/chrome/browser/tabs/tab.mm [modify] https://crrev.com/55217cf9efff8866160932c1614077d4ed43a2d3/ios/chrome/browser/tabs/tab_unittest.mm [modify] https://crrev.com/55217cf9efff8866160932c1614077d4ed43a2d3/ios/web/web_state/ui/crw_web_controller.mm [modify] https://crrev.com/55217cf9efff8866160932c1614077d4ed43a2d3/ios/web/web_state/ui/crw_web_controller_unittest.mm [modify] https://crrev.com/55217cf9efff8866160932c1614077d4ed43a2d3/ios/web/web_state/ui/crw_wk_navigation_states.h [modify] https://crrev.com/55217cf9efff8866160932c1614077d4ed43a2d3/ios/web/web_state/ui/crw_wk_navigation_states.mm [modify] https://crrev.com/55217cf9efff8866160932c1614077d4ed43a2d3/ios/web/web_state/ui/crw_wk_navigation_states_unittest.mm
,
Feb 22 2017
Verified the issue on the build 57.0.2987.72beta tested on iPhone7+(iOS 10). Most visited icons are updated correctly works fine.
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/77569373deed80a50d4787cd594f40953d1596a0 commit 77569373deed80a50d4787cd594f40953d1596a0 Author: eugenebut <eugenebut@chromium.org> Date: Thu Feb 23 19:54:34 2017 Cherry-picked remaining parts of 'Fixed title updating for back forward navigation.' Cherry-picked patchset #1 and #2 from cl/2691693002 BUG= 688047 , 677356 Review-Url: https://codereview.chromium.org/2685803002 NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2711123004 Cr-Commit-Position: refs/branch-heads/2987@{#666} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/77569373deed80a50d4787cd594f40953d1596a0/ios/web/web_state/ui/crw_web_controller.mm
,
Mar 1 2017
Verified on chrome beta version 57.0.2987.85 on iPhone 7 and iPad4 with iOS 10.2.1 following the steps mentioned in comment #0. Most visited icons are displayed correctly. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by treib@chromium.org
, Feb 2 2017