New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 896234 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Oct 25
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Cold start app displays the content suggestion article tile before loading NTP page

Project Member Reported by rakurati@chromium.org, Oct 17

Issue description

App Version: 71.0.3578.9 Beta
iOS Version: 10.3.3, 11.4.1, 12.0, 12.1 beta
Device: iPhone, iPad

Prerequisite: 
1. Enable ‘#out-of-web-fullscreen’ flag from chrome://flags
2. Enable ‘#browser-container-fullscreen’ flag from chrome://flags
 
Steps to reproduce:
1. Launch chrome app and set the above mentioned flags
2. Open New tab page
3. Cold start the app

Observed results:
Notice that before displaying NTP, article suggestion tile is displayed for just a second

Expected results:
Article suggestion tile shouldn’t be displayed before loading NTP

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: Not tested
Bug reproducible on Safari/Firefox: Firefox: No, Safari: No
Bug reproducible on current stable build (App Version, iOS Version): NA on M69 
Bug reproducible on the current beta channel build (App Version, iOS Version): Yes on M71 Beta (Feature available from M71)

Link to video:
https://drive.google.com/file/d/1QRT5aK45ndPUrN0ck6ZEM870C_hjMyAZ/view?usp=sharing

 
Cc: gambard@chromium.org justincohen@chromium.org
Labels: -Pri-2 ReleaseBlock-Stable M-71 Pri-1
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
Cc: -gambard@chromium.org kkhorimoto@chromium.org
Owner: gambard@chromium.org
Gauthier, this looks like interaction between the NTP and your browser container refactoring.  Do you want to take a look?
Labels: zine-triaged
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44a76757670054f6baafc79f37009ce8e8eaceb5

commit 44a76757670054f6baafc79f37009ce8e8eaceb5
Author: Gauthier Ambard <gambard@chromium.org>
Date: Wed Oct 24 12:53:44 2018

[iOS] Fix 0-sized NTP on cold start

When the app is launched after a cold start, the size of the collection
view is 0, even if the size of the wrapper view isn't 0. This CL fixes
it by setting the frame of the collectionView if it is 0.
This only seems to happen when the NTP is contained in a native view.

Bug:  896234 
Change-Id: I4309bf35c0a3d82e2a5c6c78265f8293ff65a43c
Reviewed-on: https://chromium-review.googlesource.com/c/1297360
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602313}
[modify] https://crrev.com/44a76757670054f6baafc79f37009ce8e8eaceb5/ios/chrome/browser/ui/content_suggestions/BUILD.gn
[modify] https://crrev.com/44a76757670054f6baafc79f37009ce8e8eaceb5/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm

Labels: Merge-Request-71
Status: Fixed (was: Assigned)
Fixed and verified in Canary.
Merge request.
Cc: kariahda@chromium.org
+kariahda@ for merge request.
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 25

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 Deleted

How confident are you in this fix? Can it brake something else?
The fix is low risk. I don't expect bad side effects.
Labels: -Merge-Review-71 Merge-Approved-71
Approved.
Labels: -Merge-Approved-71 Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/34f3c176c3da314cfbfe86788ece64b7864d8e5c

Commit: 34f3c176c3da314cfbfe86788ece64b7864d8e5c
Author: gambard@chromium.org
Commiter: gambard@chromium.org
Date: 2018-11-05 12:12:52 +0000 UTC

[iOS] Fix 0-sized NTP on cold start

When the app is launched after a cold start, the size of the collection
view is 0, even if the size of the wrapper view isn't 0. This CL fixes
it by setting the frame of the collectionView if it is 0.
This only seems to happen when the NTP is contained in a native view.

Bug:  896234 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I3925a89076095192a8f13c75ceddc76a93f03350
Reviewed-on: https://chromium-review.googlesource.com/c/1316775
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#494}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 5

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34f3c176c3da314cfbfe86788ece64b7864d8e5c

commit 34f3c176c3da314cfbfe86788ece64b7864d8e5c
Author: Gauthier Ambard <gambard@chromium.org>
Date: Mon Nov 05 12:12:52 2018

[iOS] Fix 0-sized NTP on cold start

When the app is launched after a cold start, the size of the collection
view is 0, even if the size of the wrapper view isn't 0. This CL fixes
it by setting the frame of the collectionView if it is 0.
This only seems to happen when the NTP is contained in a native view.

Bug:  896234 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I3925a89076095192a8f13c75ceddc76a93f03350
Reviewed-on: https://chromium-review.googlesource.com/c/1316775
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#494}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/34f3c176c3da314cfbfe86788ece64b7864d8e5c/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm

Verified in 
Version: 71.0.3578.39 Beta
Devices: iPhoneX (iOS 12.1 Beta), iPad 2018(iOS 11.4.1), iPhone 7(iOS 11.4.1)

Article suggestion tile is not displayed anymore before loading NTP, looks good. 

Verified on 71.0.3578.41 Beta,  iPhoneX iOS 11.4.1, iPhone6 Plus iOS10.3.3

Looks good.

Sign in to add a comment