[Search] Implementation of the invisible focus on Windows

chrome/browser/ui/views/omnibox/omnibox_view_win.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/omnibox/omnibox_view_win.h"
 
 #include <algorithm>
 #include <locale>
 #include <string>
 
@@ skipping 741 matching lines @@
   //   * The caret/selection isn't at the end of the text
   //   * The user has just pasted in something that replaced all the text
   //   * The user is trying to compose something in an IME
   CHARRANGE sel;
   GetSel(sel);
   model()->StartAutocomplete(sel.cpMax != sel.cpMin,
                             (sel.cpMax < GetTextLength()) || IsImeComposing());
 }
 
 void OmniboxViewWin::SetFocus() {
+  // Restore caret visibility if focused explicitly. We need to do this here
+  // because if we already have invisible focus, the ::SetFocus() call below
+  // will short-circuit, preventing us from reaching

[Peter Kasting, 2012/12/05 02:28:50]

You did verify this short-circuiting occurs on Windows, right?  I just want to
make sure we actually need this.

[Mathieu, 2012/12/05 03:57:21]

Modified the comment to better reflect that in certain cases (such as when a new
tab is created, OnSetFocus is not called.

+  // OmniboxEditModel::OnSetFocus(), which handles restoring visibility when we
+  // didn't previously have focus.
   ::SetFocus(m_hWnd);
+  model()->SetCaretVisibility(true);

[Peter Kasting, 2012/12/05 02:28:50]

Nit: This should be next to the comment.

[Mathieu, 2012/12/05 03:57:21]

Done.

 }
 
-void OmniboxViewWin::ApplyFocusVisibility() {
-  // TODO(mathp): implement for Windows.
-  NOTIMPLEMENTED();
+void OmniboxViewWin::ApplyCaretVisibility() {
+  // We hide the caret just before destroying it, since destroying a caret that
+  // is in the "solid" phase of its blinking will leave a solid vertical bar.
+  // We even hide and destroy the caret if we're going to create it again below.
+  // If the caret was already visible on entry to this function, the
+  // CreateCaret() call (which first destroys the old caret) might leave a solid
+  // vertical bar for the same reason as above.  Unconditionally hiding prevents
+  // this.  The caret could be visible on entry to this function if the
+  // underlying edit control had re-created it automatically (see comments in
+  // OnPaint()).
+  HideCaret();
+  // We use DestroyCaret()/CreateCaret() instead of simply HideCaret()/
+  // ShowCaret() because HideCaret() is not sticky across paint events, e.g. a
+  // window resize will effectively restore caret visibility, regardless of
+  // whether HideCaret() was called before. While we do catch and handle these
+  // paint events (see OnPaint()), it doesn't seem to be enough to simply call
+  // HideCaret() while handling them because of the unpredictability of this
+  // Windows API. According to the documentation, it should be a cumulative call
+  // e.g. 5 hide calls should be balanced by 5 show calls. We have not found
+  // this to be true, which may be explained by the fact that this API is called
+  // internally in Windows, as well.
+  ::DestroyCaret();
+  if (model()->is_focus_visible()) {
+    ::CreateCaret(m_hWnd, (HBITMAP) NULL, 1, font_.GetHeight());
+    // According to the Windows API documentation, a newly created caret needs
+    // ShowCaret to be visible.
+    ShowCaret();
+  }
 }
 
 void OmniboxViewWin::SetDropHighlightPosition(int position) {
   if (drop_highlight_position_ != position) {
     RepaintDropHighlight(drop_highlight_position_);
     drop_highlight_position_ = position;
     RepaintDropHighlight(drop_highlight_position_);
   }
 }
 
@@ skipping 903 matching lines @@
   // Modifying the selection counts as accepting any inline autocompletion, so
   // track "changes" made by clicking the mouse button.
   ScopedFreeze freeze(this, GetTextObjectModel());
   OnBeforePossibleChange();
   DefWindowProc(WM_LBUTTONDOWN, keys,
                 MAKELPARAM(ClipXCoordToVisibleText(point.x, is_triple_click),
                            point.y));
   OnAfterPossibleChange();
 
   gaining_focus_.reset();
+
+  // Restore caret visibility whenever the user clicks in the the omnibox. This
+  // is not always covered by OnSetFocus() because when clicking while the
+  // omnibox has invisible focus does not trigger a new OnSetFocus() call.
+  model()->SetCaretVisibility(true);

[Peter Kasting, 2012/12/05 02:28:50]

A couple things.

(1) Left clicks aren't enough, you need right clicks too.
(2) You also need to simulate the "gaining focus" behavior so we modify the
selection properly.

I think you may be able to achieve both of these by removing the call here and
instead modifying OnMouseActivate() to:
(1) Check "if ((!model()->has_focus() || !model()->is_caret_visible()) && ..."
(2) Place this call inside that conditional, next to the setting of
|gaining_focus_|.

You should test this to verify that it works.  You should probably try to rig up
a test environment like follows:

* Every n seconds, we ask the omnibox to make its focus invisible
* Then, test with the omnibox empty; containing text where the caret was
previously at the beginning/middle/end; and containing text with a partial and a
complete selection
* Test left/right/middle clicks, drags, double and triple clicks

[Mathieu, 2012/12/05 03:57:21]

Your proposal was almost correct; I need to check if it already has focus
(invisible focus) and the proposed "if" clause would have not done the trick.
Let me know what you think of this implementation.

As far as testing, I have tested this with left click and right click, triple
and double clicks. Dragging something on the omnibox does not activate this
event. I can't reproduce a middle click on this RDP setup (I'm at home at the
moment). I have not implemented the "invisible focus every n seconds"  test
because we should never have text in the omnibox and have invisible focus, and
would like to worry about these cases in another effort if possible. If there is
an essential case I missed or if you disagree, let me know.

 }
 
 void OmniboxViewWin::OnLButtonUp(UINT keys, const CPoint& point) {
   // default processing should happen first so we can see the result of the
   // selection
   ScopedFreeze freeze(this, GetTextObjectModel());
   DefWindowProc(WM_LBUTTONUP, keys,
                 MAKELPARAM(ClipXCoordToVisibleText(point.x, false), point.y));
 
   SelectAllIfNecessary(kLeft, point);
@@ skipping 191 matching lines @@
 
   // Draw the drop highlight.
   if (drop_highlight_position_ != -1)
     DrawDropHighlight(memory_dc, rect, paint_clip_rect);
 
   // Blit the memory DC to the actual paint DC and clean up.
   BitBlt(paint_dc, rect.left, rect.top, rect.Width(), rect.Height(), memory_dc,
          rect.left, rect.top, SRCCOPY);
   memory_dc.SelectBitmap(old_bitmap);
   edit_hwnd = old_edit_hwnd;
+
+  // This needs to be called regardless of the current state of the caret, even
+  // if reaffirming a current state (hidden or shown). This is because the
+  // underlying edit control will automatically re-create the caret when it
+  // receives a paint event, e.g. a window resize event.

[Peter Kasting, 2012/12/05 02:28:50]

A resize event and a paint event aren't the same thing -- maybe you mean "when
it receives certain events that trigger repaints, e.g. window resize events"?

[Mathieu, 2012/12/05 03:57:21]

Done.

+  ApplyCaretVisibility();
 }
 
 void OmniboxViewWin::OnPaste() {
   // Replace the selection if we have something to paste.
   const string16 text(GetClipboardText());
   if (!text.empty()) {
     // Record this paste, so we can do different behavior.
     model()->on_paste();
     // Force a Paste operation to trigger the text_changed code in
     // OnAfterPossibleChange(), even if identical contents are pasted into the
@@ skipping 839 matching lines @@
   return (rect.left - client_rect.left) + (client_rect.right - rect.right);
 }
 
 int OmniboxViewWin::WidthNeededToDisplay(const string16& text) const {
   // Use font_.GetStringWidth() instead of
   // PosFromChar(location_entry_->GetTextLength()) because PosFromChar() is
   // apparently buggy. In both LTR UI and RTL UI with left-to-right layout,
   // PosFromChar(i) might return 0 when i is greater than 1.
   return font_.GetStringWidth(text) + GetHorizontalMargin();
 }