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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months ago by mthiesse
Modified:
4 months, 2 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 #

Messages

Total messages: 18 (8 generated)
mthiesse
ymalik, asimjour: Please review VrShellImpl changes. tedchoc: Please review omnibox/ and ntp/ changes.
5 months 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 ...
5 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 ...
5 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: ...
4 months, 3 weeks ago (2017-05-31 21:27:11 UTC) #7
mthiesse
ping all :)
4 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 ...
4 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 ...
4 months, 3 weeks ago (2017-06-02 18:27:21 UTC) #10
Ted C
lgtm
4 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
4 months, 3 weeks ago (2017-06-02 19:05:25 UTC) #15
commit-bot: I haz the power
4 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa