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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by mthiesse
Modified:
3 weeks, 1 day 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.
1 month, 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 ...
1 month 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 month 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: ...
3 weeks, 5 days ago (2017-05-31 21:27:11 UTC) #7
mthiesse
ping all :)
3 weeks, 4 days 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 ...
3 weeks, 4 days 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 ...
3 weeks, 3 days ago (2017-06-02 18:27:21 UTC) #10
Ted C
lgtm
3 weeks, 3 days 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
3 weeks, 3 days ago (2017-06-02 19:05:25 UTC) #15
commit-bot: I haz the power
3 weeks, 3 days 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 cb946e318