Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2153)

Issue 2891423002: Disable NTP URL bar while in VR on Android. (Closed)

Created:
1 year ago by mthiesse
Modified:
1 year ago
Reviewers:
Ted C, Bernhard Bauer
CC:
chromium-reviews, jdonnelly+watch_chromium.org, feature-vr-reviews_chromium.org, noyau+watch_chromium.org, ntp-dev+reviews_chromium.org, agrieve+watch_chromium.org, asimjour1
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Disable NTP URL bar while in VR on Android. Unfortunately we don't support keyboard or microphone input while in VR just yet, so we need to disable them to prevent the 2D popups from obscuring VR content. BUG=724551 Review-Url: https://codereview.chromium.org/2891423002 Cr-Commit-Position: refs/heads/master@{#476779} Committed: https://chromium.googlesource.com/chromium/src/+/c8c17ea5f62b4d647f2cb4e892333927d8e37779

Patch Set 1 #

Patch Set 2 : cleanup #

Patch Set 3 : javadoc #

Total comments: 3

Patch Set 4 : Make focusSearchBox aware of VR #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -0 lines) Patch
M chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java View 1 2 3 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 18 (8 generated)
mthiesse
ymalik, asimjour: Please review VrShellImpl changes. tedchoc: Please review omnibox/ and ntp/ changes.
1 year ago (2017-05-19 17:54:03 UTC) #3
Ted C
+bauerb for ntp thoughts https://codereview.chromium.org/2891423002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java (right): https://codereview.chromium.org/2891423002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java#newcode433 chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java:433: mSearchProviderHasLogo = TemplateUrlService.getInstance().isDefaultSearchEngineGoogle(); Is it ...
1 year ago (2017-05-23 14:37:46 UTC) #5
mthiesse
Amir or Yash, feel free to pick this CL up from me while I'm out ...
1 year ago (2017-05-23 15:50:24 UTC) #6
mthiesse
https://codereview.chromium.org/2891423002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java (right): https://codereview.chromium.org/2891423002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java#newcode433 chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java:433: mSearchProviderHasLogo = TemplateUrlService.getInstance().isDefaultSearchEngineGoogle(); On 2017/05/23 14:37:45, Ted C wrote: ...
1 year ago (2017-05-31 21:27:11 UTC) #7
mthiesse
ping all :)
1 year ago (2017-06-02 14:49:36 UTC) #8
Ted C
https://codereview.chromium.org/2891423002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java (right): https://codereview.chromium.org/2891423002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java#newcode161 chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPage.java:161: void setUrlBarFocusable(boolean focusable); I feel there needs to be ...
1 year ago (2017-06-02 17:01:51 UTC) #9
mthiesse
Ah, thanks for the pointer to focusSearchBox. I think just returning early there is more ...
1 year ago (2017-06-02 18:27:21 UTC) #10
Ted C
lgtm
1 year ago (2017-06-02 19:04:07 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2891423002/60001
1 year ago (2017-06-02 19:05:25 UTC) #15
commit-bot: I haz the power
1 year ago (2017-06-02 20:54:28 UTC) #18
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/c8c17ea5f62b4d647f2cb4e89233...

Powered by Google App Engine
This is Rietveld 408576698