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

Issue 807957 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Omnibox cannot be scrolled down in Aw Snap page

Project Member Reported by pmadalla@chromium.org, Feb 1 2018

Issue description

App Version: 66.0.3335.0 canary
iOS Version: 11.2.2
Device : iPhones

Steps to reproduce : 
1. Launch chrome.
2. Goto chrome://crash
3. Swipe the Aw Snap page to full screen.
4. Try to scroll down the page to get the omnibox
 
Observed results:
Omnibox cannot be scrolled down in Aw Snap page

Expected results:
Omnibox cannot Should be scrolled down in Aw Snap page

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 Dolphin/Safari/Firefox: Safari : NA
Bug reproducible on current stable build (App Version, iOS Version): No in M64
Bug reproducible on the current beta channel build (App Version, iOS Version): No in M65

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

 
Cc: sczs@chromium.org justincohen@chromium.org
Labels: Bijou-Fullscreen
Owner: kkhorimoto@chromium.org
Status: Assigned (was: Untriaged)
Status: Started (was: Assigned)
crrev.com/c/896304 prevents the toolbar from being scrolled away, but also breaks some scrolling behavior for this page.  I'll continue to work on getting that error page to scroll properly after that lands.
Just tried disabling the fullscreen flag, and the old implementation is even more broken than the current version.  The only issue with the new version (now that crrev.com/c/896304 is landed) is that scrolling can sometimes be disabled after interacting with the page.  Definitely not release-blocking, but I will continue to investigate a fix.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 8 2018

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

commit 963abca215a516aab5fbe81e17fb96466d327222
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Thu Feb 08 18:01:23 2018

[iOS] Improve FullscreenModel scroll handling.

This CL improves fullscreen handling for several edge
conditions.  The behavior described in the referenced bug
is due to FullscreenModel attempting to handle UIScrollView
adjustments that occur when bouncing past the top edge of
content.  Similarly broken behavior also occurred when
attempting to scroll pages smaller or similarly sized with
the scroll view.

After this change, FullscreenModel ignores scrolls that
have to do with UIScrollView adjusting its contentOffset
for bouncing or resizing behavior.

Implementing this logic also required exposing the
scroll view frame, contentInset, contentSize, and zooming
properties.

Bug: 800757,  807957 , 809853, 809856
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I8980983e048ce73d3443160cb4dd2e29a1aed15a
Reviewed-on: https://chromium-review.googlesource.com/896304
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535435}
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/broadcaster/chrome_broadcast_observer.h
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/broadcaster/chrome_broadcast_observer_bridge.h
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/broadcaster/chrome_broadcast_observer_bridge.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/broadcaster/chrome_broadcaster.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/broadcaster/chrome_broadcaster_unittest.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/fullscreen/fullscreen_controller_impl.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/fullscreen/fullscreen_model.h
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/fullscreen/fullscreen_model.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/fullscreen/fullscreen_model_unittest.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/fullscreen/test/fullscreen_model_test_util.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/main_content/main_content_ui_broadcasting_util.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/main_content/main_content_ui_state.h
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/main_content/main_content_ui_state.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/chrome/browser/ui/main_content/web_scroll_view_main_content_ui_forwarder.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/web/public/web_state/ui/crw_web_view_scroll_view_proxy.h
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/web/web_state/ui/crw_web_view_scroll_view_proxy.mm
[modify] https://crrev.com/963abca215a516aab5fbe81e17fb96466d327222/ios/web/web_state/ui/crw_web_view_scroll_view_proxy_unittest.mm

Cc: pmadalla@chromium.org
Status: Fixed (was: Started)
crrev.com/c/896304 has landed, which fixes this issue and others.  We probably want this fix in M65, but it's a rather large change, so we don't want to cherry-pick immediately.  pmadalla@, could you take a look in tomorrow's canary?
Status: Assigned (was: Fixed)

Issue can be reproduced with below steps

Steps to repro :
1.Launch chrome.
2.Goto chrome://crash.
3.Tap on Menu > New incognito tab.
4.Tap on tab-switcher and navigate back to normal tab.
5.Swipe the Aw-Snap message to full screen.

Observed results:
Omnibox cannot be scrolled down in Aw Snap page

Expected results:
Omnibox cannot Should be scrolled down in Aw Snap page

Video : 
https://drive.google.com/file/d/1iSHCDCceyiP2gf0nQwieMw3f-cMiCItm/view?usp=sharing

Retested the issue on latest canary 66.0.3344.0 tested on iPhone7+(iOS 11.2.5),iPhoneX(iOS 11beta3),iPhone6 (iOS10.3.5).


Status: Started (was: Assigned)
Whoops, I forgot to remove this issue from the CL description.  This was NOT fixed by that CL that landed; I'm still working on a fix for this.
Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 16 2018

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

commit e8bbb37f4c9a26f147c5623ef88dd7828a53d83b
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Fri Feb 16 23:15:06 2018

[iOS] Disable fullscreen for SadTab.

Additionally, this CL updates CRWGenericContentView's contentSize
adjustment logic to ensure that scrolling doesn't get disabled.

Bug:  807957 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Idf48f9b79966520f31477b0a58415eb286f037ae
Reviewed-on: https://chromium-review.googlesource.com/917642
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537450}
[modify] https://crrev.com/e8bbb37f4c9a26f147c5623ef88dd7828a53d83b/ios/chrome/browser/web/BUILD.gn
[modify] https://crrev.com/e8bbb37f4c9a26f147c5623ef88dd7828a53d83b/ios/chrome/browser/web/sad_tab_tab_helper.h
[modify] https://crrev.com/e8bbb37f4c9a26f147c5623ef88dd7828a53d83b/ios/chrome/browser/web/sad_tab_tab_helper.mm
[modify] https://crrev.com/e8bbb37f4c9a26f147c5623ef88dd7828a53d83b/ios/web/web_state/ui/crw_generic_content_view.mm

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 22 2018

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

commit c8f2b8620d74363c2c0bf1a330d0f6a46850494a
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Thu Feb 22 20:41:29 2018

[iOS] Improve FullscreenModel scroll handling.

This CL improves fullscreen handling for several edge
conditions.  The behavior described in the referenced bug
is due to FullscreenModel attempting to handle UIScrollView
adjustments that occur when bouncing past the top edge of
content.  Similarly broken behavior also occurred when
attempting to scroll pages smaller or similarly sized with
the scroll view.

After this change, FullscreenModel ignores scrolls that
have to do with UIScrollView adjusting its contentOffset
for bouncing or resizing behavior.

Implementing this logic also required exposing the
scroll view frame, contentInset, contentSize, and zooming
properties.

TBR=kkhorimoto@chromium.org

(cherry picked from commit 963abca215a516aab5fbe81e17fb96466d327222)

Bug: 800757,  807957 , 809853, 809856
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I8980983e048ce73d3443160cb4dd2e29a1aed15a
Reviewed-on: https://chromium-review.googlesource.com/896304
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#535435}
Reviewed-on: https://chromium-review.googlesource.com/932548
Cr-Commit-Position: refs/branch-heads/3325@{#555}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/broadcaster/chrome_broadcast_observer.h
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/broadcaster/chrome_broadcast_observer_bridge.h
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/broadcaster/chrome_broadcast_observer_bridge.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/broadcaster/chrome_broadcaster.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/broadcaster/chrome_broadcaster_unittest.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/fullscreen/fullscreen_controller_impl.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/fullscreen/fullscreen_model.h
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/fullscreen/fullscreen_model.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/fullscreen/fullscreen_model_unittest.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/fullscreen/test/fullscreen_model_test_util.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/main_content/main_content_ui_broadcasting_util.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/main_content/main_content_ui_state.h
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/main_content/main_content_ui_state.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/chrome/browser/ui/main_content/web_scroll_view_main_content_ui_forwarder.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/web/public/web_state/ui/crw_web_view_scroll_view_proxy.h
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/web/web_state/ui/crw_web_view_scroll_view_proxy.mm
[modify] https://crrev.com/c8f2b8620d74363c2c0bf1a330d0f6a46850494a/ios/web/web_state/ui/crw_web_view_scroll_view_proxy_unittest.mm

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 23 2018

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

commit b849bd3366044dcaff2912878d8605be2f27573f
Author: Kurt Horimoto <kkhorimoto@chromium.org>
Date: Fri Feb 23 00:33:29 2018

[iOS] Disable fullscreen for SadTab.

Additionally, this CL updates CRWGenericContentView's contentSize
adjustment logic to ensure that scrolling doesn't get disabled.

Bug:  807957 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Idf48f9b79966520f31477b0a58415eb286f037ae
Reviewed-on: https://chromium-review.googlesource.com/917642
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Justin Cohen <justincohen@chromium.org>
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#537450}(cherry picked from commit e8bbb37f4c9a26f147c5623ef88dd7828a53d83b)
Reviewed-on: https://chromium-review.googlesource.com/933304
Cr-Commit-Position: refs/branch-heads/3325@{#563}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/b849bd3366044dcaff2912878d8605be2f27573f/ios/chrome/browser/web/BUILD.gn
[modify] https://crrev.com/b849bd3366044dcaff2912878d8605be2f27573f/ios/chrome/browser/web/sad_tab_tab_helper.h
[modify] https://crrev.com/b849bd3366044dcaff2912878d8605be2f27573f/ios/chrome/browser/web/sad_tab_tab_helper.mm
[modify] https://crrev.com/b849bd3366044dcaff2912878d8605be2f27573f/ios/web/web_state/ui/crw_generic_content_view.mm

Status: Verified (was: Fixed)
verfied the issue on the build version 66.0.3356.0 canary tested on iPhone7+(iOS 11.)

Fix provided :
Unable to scroll omnibox to full screen,works fine
Verified the issue on 65.0.3325.103 Beta on iPhone 8plus(iOS 11.2.6) and iPhone 6s plus(iOS 10.3.3)

Looks good page doesn't go to full screen on following steps mentioned in comment #0 and comment #7

Sign in to add a comment