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

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

Created:
11 months, 1 week ago by mthiesse
Modified:
10 months, 3 weeks 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.
11 months, 1 week 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 ...
11 months 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 ...
11 months 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: ...
10 months, 3 weeks ago (2017-05-31 21:27:11 UTC) #7
mthiesse
ping all :)
10 months, 3 weeks 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 ...
10 months, 3 weeks 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 ...
10 months, 3 weeks ago (2017-06-02 18:27:21 UTC) #10
Ted C
lgtm
10 months, 3 weeks 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
10 months, 3 weeks ago (2017-06-02 19:05:25 UTC) #15
commit-bot: I haz the power
10 months, 3 weeks 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