New issue
Advanced search Search tips

Issue 879935 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

[MdRefresh] Tab-To-Search icon becomes a visual glitch when switching between tabs/windows/applications

Project Member Reported by meh...@chromium.org, Sep 2

Issue description

Chrome Version: Canary 71.0.3540.0
OS: macOS, but probably OS=ALL

What steps will reproduce the problem?
(1) Type youtube into the Omnibox, so that the Tab-To-Search icon appears in the Omnibox
(2) Switch away from that tab/window to another tab or window or application 
(3) Switch back to the tab where Tab-To-Search icon was shown before

What is the expected result?
A sharp and rounded icon like shown before.

What happens instead?
The icon becomes blurry with squared corners.

A screencast and screenshots are attached.

Thanks
Mehmet
 
actual.png
28.5 KB View Download
expected.png
28.2 KB View Download
Tab-To-Search-icon-issue.mov
6.6 MB View Download
I guess the "Tab" key image is being drawn with the darker shade of gray as background when Chrome window loses focus, then this composite image is drawn without being invalidated/refreshed when focus is regained.  I haven't looked at the code yet, though, this is just first speculation.
Labels: Hotlist-Polish
[omnibox triage]

orinj@, want to become the owner of this bug? :-)

Owner: orinj@chromium.org
Status: Started (was: Untriaged)
Haha, sure!  I will take a look today. :)
The problem is this:
* Background color selected for "chip_container" (the "icon") depends on whether the omnibox popup is open.  If open, white; otherwise, gray.
* When switching applications, notice that the popup closes.  It does not re-open when Chrome window is activated.

I see a few possible solutions:
* Correct or eliminate the condition that changes background color (popup presence/absence seems wrong).
* Don't show this keyword_hint_view when the popup is absent.  Without the popup, it isn't clear what's going on anyway, and continuing to type will restore the popup as well as this view (with correct white background).
* Restore the omnibox state more completely when Chrome window reactivates.  Open the popup and this issue then gets solved for free.

I like the last option ideally, but it also seems the most likely to cause other side effects.  Is there a reason we aren't restoring omnibox popup state when Chrome window reactivates?  Hiding it on deactivation makes some sense, but it's odd to come back to an omnibox that is focused but isn't fully awake until user continues typing.

Thoughts?
One concern:
- the omnibox has some hidden state, such as whether the last character typed was a backspace.  (This causes the system to prevent inline autocompletions.)  None of this state is saved or restored.  Thus, if we reopen the dropdown when the user leaves and returns, they may end up seeing different suggestions than they saw before, which is confusing.

Interesting.  Well how about leaving the autocomplete results (and hence the popup) intact instead of clearing them on window deactivation?  I already have a "fix" working by changing this true to false:
https://cs.chromium.org/chromium/src/components/omnibox/browser/omnibox_edit_model.cc?rcl=b11671ddb4d5fd0d7de007d0d4dd894e85238796&l=457

This change is far too effective, and also keeps the popup around when it should go away.  But if I can properly prevent autocomplete results from getting blasted on window switch then it's a softer switch and the user would come back to a ready working omnibox just as it was before.  Coming back to a half-filled focused edit box with popup lost until keystroke already seems pretty confusing to me.

This feels like a bit of a danger zone, though -- do we really need to clear results and close the popup when switching windows?  Sure it's easy/safe, but is it the best experience for users?

This is by far the most complex OnBlur method I've ever seen:
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/omnibox/omnibox_view_views.cc?rcl=93db813ea6170ae8a77b56287d4cdcd442e7969c&l=1057

But there's a difference between blurring omnibox within Chrome versus deactivating Chrome window as a whole.  Which brings me to a more general question that may guide the course here:

Shouldn't we be restoring omnibox/popup/autocomplete-results state when the user re-focuses the omnibox?  Defaulting to full selection seems fine, but suppose the user presses right-arrow or End key -- if the user comes to a state where they can type, shouldn't they be getting the suggestions for what's entered so far?  If we get that, then all the above save/restore/window-reactivation complexity goes away.
Cc: pkasting@chromium.org
+pkasting@ for thoughts?  If there's a good reason to not pick up with a new autocomplete session when user enters the state where typing a character would resume autocomplete -- then I can drop this and do the simple fix of changing background color.  But I feel like this is an opportunity to improve the user experience by resuming where they left off with autocomplete & suggestions restored as soon as the input is ready for new keystrokes.  It's definitely the more complex solution to this simple bug, but I think has a good impact.
Popup closing when the omnibox loses focus or the window loses activation is intentional and I wouldn't change that.  You should do comment 4 bullet 3.

I have more thoughts on why restoring the popup state isn't actually what you want, but I'll elide most, except to say that often you restore activation by doing something which will either change the cursor position or unfocus the omnibox anyway, so you don't want the state restored then anyway.
Okay, thanks.  Interpreting "bullet 3" with 1-based counting from top of comment (not solutions list) -- eliminating the condition appears to be the easiest approach to fix this bug.  Will do!

Just to clarify, though, in case the popup restoration issue comes up again someday: when I talk about restoring state, I don't mean forcing omnibox wholly back to what it was before, but simply picking up the autocomplete session to bring back suggestions list without requiring a new keystroke.  We can detect when the edit input is in a mode to receive new keystrokes which would trigger popup anyway, and I just think it would be nice to go ahead and bring it up without requiring those keystrokes -- only when edit cursor is ready to modify existing input.  If the user clicks to select all or changes cursor position, then that may prevent the popup because that's a state change within omnibox context.  I agree that changing window activation state *often* happens in a way that changes omnibox state (clicking window, e.g.), but this isn't necessarily so.  Alt+Tab window switching is an example where it'd be nice to be able to pick up where we left off.  Closing the popup seems safe and reasonable, but bringing it back in this case seems consistent and convenient.  It's not a big deal, though: it works well enough as it is and requiring a keystroke isn't terribly confusing.  No need to convince me with the elided thoughts - the fact that you have some reasons is good enough for me, thanks! :)
I misunderstood your list, but you settled on the right answer.  The issue is that the color is being chosen based on whether the popup is open, when in fact the omnibox background color changes based on whether the omnibox is focused.

I don't know where the code in question is, but we ought to be able to simply ask "give me the current omnibox background color" and have it return the right thing, instead of writing manual conditionals to try to match code elsewhere (if that's what's happening).
pkasting: I'm still curious about the answer to orinj's question. Namely, it'd it be useful when the user alt-tabbed out and back to have the popup still open. I imagine that the current behavior is based on hard-won experience and you don't need to provide a full history or explanation. Is it just the complexity that would be involved in trying to preserve that state on focus loss? Or is there a different scenario in which keeping it open is counterproductive?
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91e0b5da4fd6efc306339ecffaf9429accd483ab

commit 91e0b5da4fd6efc306339ecffaf9429accd483ab
Author: Orin Jaworski <orinj@chromium.org>
Date: Fri Sep 07 18:00:41 2018

[omnibox] Fix color on keyword hint view chip label & container

The keyword hint view had a chip container with "Tab" label,
and the background color assigned was incorrectly conditioned
on whether the popup is open.  This hint doesn't show when
omnibox is unfocused, and only OmniboxPart::RESULTS_BACKGROUND
gives the correct color so it is used directly instead of logic.

Bug:  879935 
Change-Id: I29b157054c30f6ef72d44169ef1539dfd0bfc9a5
Reviewed-on: https://chromium-review.googlesource.com/1211963
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Reviewed-by: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589579}
[modify] https://crrev.com/91e0b5da4fd6efc306339ecffaf9429accd483ab/chrome/browser/ui/views/location_bar/keyword_hint_view.cc
[modify] https://crrev.com/91e0b5da4fd6efc306339ecffaf9429accd483ab/chrome/browser/ui/views/location_bar/keyword_hint_view.h
[modify] https://crrev.com/91e0b5da4fd6efc306339ecffaf9429accd483ab/chrome/browser/ui/views/location_bar/location_bar_view.cc

@11: Sorry it took so long for me to reply.

The core problem to me is the difference between what the code calls the user text and the temporary text.  From a user perspective, this is the concept that "I've typed some stuff, and if I arrow/tab around, the omnibox temporarily shows something else, but I can still hit esc to cancel that and get back to what I was typing".  Closing the popup, like several other actions (typing another character, hitting enter), "commits" any temporary text as "the thing you've typed", and thus should change the items in the dropdown, as well as what should happen when you hit esc.  This is how autofill popups and similar UI elements elsewhere on the system behave as well.

So if you deactivate the window and the popup closes, then reactivate the window, restoring the state exactly as it was doesn't always make sense, because if there was temporary text, the closure should have committed it.  Plus, reopening the popup with a selection halfway down feels weird.  But reopening the popup with different suggestions and a different selected line is also strange.

You can get around a lot of this by not closing the omnibox popup when the window is deactivated.  But that is VERY strange in terms of how UI elements are expected to interact; the omnibox popup is intended to feel transient and ephemeral, not like a child dialog.  Not closing when switching to another window would probably have users up in arms.  In that case you also couldn't really justify closing the popup when focusing the web content area, and that would break legit use cases.  And if this really is "ephemeral", then not only should it close when we deactivate, it shouldn't auto-reopen anyway, because that's not what transient things do.

So I think it feels most consistent and coherent to have deactivation work like focus loss; have focus loss close the popup; and have closing the popup commit any temporary text.
That all makes a lot of sense, thanks for laying out the logic from a code perspective - and I think the way it is seems to be good enough for a whole lot of happy users!  I am probably a rare kind of user in the way I think about this... to me, the omnibox is like a text input and when I'm in the mode of inserting/appending text to what's there (empty or not), I like to see the full state of what I'm working with, right from the start.  So even without switching windows, simply focusing web content area and then re-focusing omnibox, I'd like the popup to return without me having to actually modify the query with another keystroke to get it back.  Once you get past the all-text-selected state, now it's really obvious that the text input is ready for insert/append, and the behavior is the same whether I'm at new-keystroke #0, #1, #2, ... right now, #0 is exceptional, for a fair share of reasons.  It's not bad, though - I understand the omnibox is much more than a text input, and for something that does so many things, there's bound to be many ways to navigate the various uses and states.  Taste and users' past experience comes into play, and this is a pretty subtle matter to begin with.
There are more bits involving cursor position changing, and whether that changes the suggestions, and so forth, but if you want to get your popup back, the easiest way is to simply hit the down arrow key, which causes it to reopen without doing anything else.
Labels: Group-Omnibox
Status: Fixed (was: Started)
Project Member

Comment 18 by bugdroid1@chromium.org, Sep 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/87a32276dd642f8cda2cdfe57f8bea5ca0d89a9e

commit 87a32276dd642f8cda2cdfe57f8bea5ca0d89a9e
Author: Orin Jaworski <orinj@chromium.org>
Date: Mon Sep 24 20:32:36 2018

Remove confusing comment from KeywordHintView::SetKeyword

This comment should have been removed with code review 1211963
because the bug fixed there was precisely the change of color
depending on popup state.

Bug:  879935 
Change-Id: Ie4dd596a96050759d45bd542a75ab06e066c3797
Reviewed-on: https://chromium-review.googlesource.com/1240203
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Orin Jaworski <orinj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593664}
[modify] https://crrev.com/87a32276dd642f8cda2cdfe57f8bea5ca0d89a9e/chrome/browser/ui/views/location_bar/keyword_hint_view.cc

Cc: vamshi.kommuri@chromium.org
Labels: Needs-Feedback
Tried checking the issue on reported chrome version 71.0.3540.0 using Mac 10.13.1 by enabling various flags (...as mentioned below) and by typing yout... in omnibox.
#top-chrome-md
#omnibox-rich-entity-suggestions
#omnibox-tab-switch-suggestions
#enable-query-in-omnibox
Yet we couldn't see any suggestions in the omnibox.

@Orin Jaworski: Please let us know if there is any pre-condition, in order to get the suggestion(s), requesting you to help us in verifying the fix.

Thanks!
I think you mean that you were unable to trigger keyword suggestions to show the keyword hint view (where the "visual glitch" was happening).  You should be able to get them if you go to google.com and search something, then open a new tab and type "google.com" followed by the [Tab] key.  You can configure other keywords in settings, but this one comes stock.  The trick is to actually go and do a search first.

Sign in to add a comment