VR Shell: Cannot interact with bottom ~1/4 of webpage. |
||||||||||
Issue descriptionNavigate to a wikipedia page in VR, exit VR, then re-enter VR. Try clicking/hovering links in the bottom 1/4 of the page and note that it doesn't work.
,
May 15 2017
,
May 15 2017
Related to issue 715182 ? Looks like exiting and re-entering VR triggers this.
,
May 22 2017
just hit a repro on canary - I'll investigate.
,
May 23 2017
We appear to be going down some of the fallback paths in PaintLayer::HitTest. I'm unable to repro in recent builds, but Canary still hits it consistently for me. I'm unable to repro in builds that had the fix for 715182, but can easily repro when I remove that fix. I'm going to resolve as fixed - same cause as 715218.
,
May 26 2017
Reactivating - this is hitting in builds with the fix for 715218.
,
May 26 2017
,
May 26 2017
I'm also having trouble interacting with the upper left corner of the screen. When visiting usatoday.com for the first time, it displays a bar asking you to install their Android app. While I can click the Install button the right side, I can't click the 'x' on the left side. I can click the hamburger menu just below it. After exiting VR, I can click the 'x'.
,
May 30 2017
When we enter webvr, we try to set the viewport to 960x640. However, this ends up turning into 960x477, to leave room for the address bar. I've lately only been able to repro on Canary rather than private builds, unless I turn on the "Chrome Home Android" setting in about:flags for private builds.
,
May 31 2017
In VrShellImpl, we call setContentCssSize to update a virtual display's size, which updates CVC's size, layout size, etc. This bubbles throughout the system, eventually making it to RenderWidgetHostViewBase, which sets the viewport size (as obtained by CVCImpl::GetViewSize(). This subtracts off the size of the controls. DoBrowserControlsShrinkBlinkSize() is unfortunately returning true while we are in VR after we set the size. We do try to hide the controls through updateFullscreenEnabledState, which starts sending async messages which start animations to actually hide the controls, but we want to hide them synchronously to make the sizing correct. In order to fix this, we need doBrowserControlsShrinkBlinkSize() to return false while in VR. We can do this by: 1. making CVC aware of VR. 2. adding methods to push an override to hide controls at the CVC level (VRShell would call these methods) 3. adding plumbing to push the override through from fullscreen manager's updateFullscreenEnabledState, so we can synchronously change state.
,
May 31 2017
Wait, so why is this value not being updated after the browser controls are hidden? The content area size should change when the controls are hidden... I think trying to make this synchronous probably wouldn't be the right solution?
,
Jun 1 2017
Please see if you can avoid adding more methods in CVC or let content layer be aware of VR mode. Fix would have been much easier if resize events had been refactored to go through ViewAndroid tree but it's still in progress Issue 722543 A quick remedy I can suggest is to have RWHVA return the adjusted size using DoBrowserControlsShrinkBlinkSize().
,
Jun 1 2017
... I meant Issue 622847
,
Jun 1 2017
mthiesse, The controls are set to animate out, and the animation doesn't play because VrShell stops vsyncs. This means that the controls are never fully hidden (problem 1). If we enable animations, the bug would repro until the animation completes. Maybe we could somehow force the animation to complete (problem 2). Finally, even if we do wait for the animation to complete, it doesn't trigger resize events throughout the stack. I haven't fully traced down whether they aren't even triggered or whether they get stopped somewhere. The size through much of the stack doesn't think the size has changed. (problem 3). I agree that the ideal fix would be: 1. Either directly trigger hiding or force the animation to complete to address problem 1 and 2. 2. Ensure that notifications throughout the stack handle viewport changing because controls were hidden. However, this looks like a more complicated/risky fix, and I'd rather merge back a simpler change, especially if there is an upcoming refactor that could address some of the complexity here.
,
Jun 1 2017
I looked at why we don't resize when the controls are hidden. It looks like the code that should do this is UpdateICBAndResizeViewport. Two interesting things. First it looks like we'll only subtract off height (so the viewport and ICB are the same whether controls are shown or not). Second, the control height is ultimately coming from the top controls height, not the bottom controls height. I'll investigate more tomorrow to see whether this is actually a cleaner fix.
,
Jun 7 2017
,
Jun 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a4ab2b3a0adc017cfe2d0ab19ad2092bf3beabb7 commit a4ab2b3a0adc017cfe2d0ab19ad2092bf3beabb7 Author: Matthew Jones <mdjones@chromium.org> Date: Tue Jun 13 00:19:11 2017 [Home] Fix vr dimensions in Chrome Home This change modifies the fullscreen manager's concept of whether or not the browser controls should modify the viewport to depend on the whether or not VR is active. BUG= 722543 , 730772 Change-Id: I00770dffe6d278232d9e8a0d7ee5dd9be9c01a87 Reviewed-on: https://chromium-review.googlesource.com/528521 Reviewed-by: Ted Choc <tedchoc@chromium.org> Commit-Queue: Matthew Jones <mdjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#478837} [modify] https://crrev.com/a4ab2b3a0adc017cfe2d0ab19ad2092bf3beabb7/chrome/android/java/src/org/chromium/chrome/browser/fullscreen/ChromeFullscreenManager.java
,
Jun 13 2017
,
Jun 23 2017
Verified in Chrome Canary 61.0.3138.0 |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by mthiesse@chromium.org
, May 15 2017