[AIM] Switch back to a focused tab puts us in STANDBY. [chromium/src : main]
Sky Malice (Gerrit)
Sky Malice voted and added 2 comments![]( "Open in Gerrit")
Votes added by Sky Malice
| Commit-Queue | +1 |
2 comments
[AIM] Switch back to a focused tap puts us in STANDBY.Sky MaliceTab?
Done
if (input.getAutocompleteState() == AutocompleteState.ENABLED) {Sky MaliceWhat other states could it be and why would we not want to switch to STANDBY in these cases?
So there are currently 4 states. DISABLED, STANDBY, ENABLED, STANDBY_NO_FOCUS. DISABLED should stay disabled. STANDBY already is STADNBY. ENABLED is the one I explicitly handle here. And STANDBY_NO_FOCUS is the odd one out.
The thing is, I don't really know what STANDBY_NO_FOCUS should be doing. I don't think we should ever get it here unless something has gone wrong. STANDBY_NO_FOCUS is mostly only used on the NTP fakebox when we're going to show the popup and we need to init machinery but not affect the UI at all. I'm hesitant to assert because it might be possible to hit. I think leaving it as is probably fine.
Related details
- Patrick Noland
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sky Malice (Gerrit)
Sky Malice added 1 comment![]( "Open in Gerrit")
if (input.getAutocompleteState() == AutocompleteState.ENABLED) {Sky MaliceWhat other states could it be and why would we not want to switch to STANDBY in these cases?
So there are currently 4 states. DISABLED, STANDBY, ENABLED, STANDBY_NO_FOCUS. DISABLED should stay disabled. STANDBY already is STADNBY. ENABLED is the one I explicitly handle here. And STANDBY_NO_FOCUS is the odd one out.
The thing is, I don't really know what STANDBY_NO_FOCUS should be doing. I don't think we should ever get it here unless something has gone wrong. STANDBY_NO_FOCUS is mostly only used on the NTP fakebox when we're going to show the popup and we need to init machinery but not affect the UI at all. I'm hesitant to assert because it might be possible to hit. I think leaving it as is probably fine.
Wait, maybe I don't even need the if at all. If we were active, we should just downgrade to standby, right?
Related details
- Patrick Noland
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Noland (Gerrit)
Patrick Noland added 1 comment![]( "Open in Gerrit")
if (input.getAutocompleteState() == AutocompleteState.ENABLED) {Sky MaliceWhat other states could it be and why would we not want to switch to STANDBY in these cases?
Sky MaliceSo there are currently 4 states. DISABLED, STANDBY, ENABLED, STANDBY_NO_FOCUS. DISABLED should stay disabled. STANDBY already is STADNBY. ENABLED is the one I explicitly handle here. And STANDBY_NO_FOCUS is the odd one out.
The thing is, I don't really know what STANDBY_NO_FOCUS should be doing. I don't think we should ever get it here unless something has gone wrong. STANDBY_NO_FOCUS is mostly only used on the NTP fakebox when we're going to show the popup and we need to init machinery but not affect the UI at all. I'm hesitant to assert because it might be possible to hit. I think leaving it as is probably fine.
Wait, maybe I don't even need the if at all. If we were active, we should just downgrade to standby, right?
You still don't want to upgrade from DISABLED to STANDBY though?
Related details
- Sky Malice
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sky Malice (Gerrit)
Sky Malice voted and added 1 comment![]( "Open in Gerrit")
Votes added by Sky Malice
| Commit-Queue | +1 |
1 comment
if (input.getAutocompleteState() == AutocompleteState.ENABLED) {Sky MaliceWhat other states could it be and why would we not want to switch to STANDBY in these cases?
Sky MaliceSo there are currently 4 states. DISABLED, STANDBY, ENABLED, STANDBY_NO_FOCUS. DISABLED should stay disabled. STANDBY already is STADNBY. ENABLED is the one I explicitly handle here. And STANDBY_NO_FOCUS is the odd one out.
The thing is, I don't really know what STANDBY_NO_FOCUS should be doing. I don't think we should ever get it here unless something has gone wrong. STANDBY_NO_FOCUS is mostly only used on the NTP fakebox when we're going to show the popup and we need to init machinery but not affect the UI at all. I'm hesitant to assert because it might be possible to hit. I think leaving it as is probably fine.
Patrick NolandWait, maybe I don't even need the if at all. If we were active, we should just downgrade to standby, right?
You still don't want to upgrade from DISABLED to STANDBY though?
In that case it should fail the state.isSessionActive check. Simplified.
Related details
- Patrick Noland
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Noland (Gerrit)
Patrick Noland voted Code-Review+1![]( "Open in Gerrit")
- Sky Malice
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[AIM] Switch back to a focused tab puts us in STANDBY.
This matches W/M/L behavior more closely, if we were previously in
ENABLED, and we switch back to a tab, while we maintain text and
selection, we downgrade to STANDBY which also avoids suggestions.
- M chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/LocationBarMediator.java
- M chrome/browser/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/LocationBarMediatorTest.java
Code-Review: +1 by Patrick Noland
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |