cocoa: allow omnibox decorations to become key

chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm

 // Copyright 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.
 
 #import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
 
 #include "base/bind.h"
 #import "base/mac/mac_util.h"
 #include "base/memory/ptr_util.h"
 #include "base/stl_util.h"
@@ skipping 724 matching lines @@
 bool LocationBarViewMac::IsSecureConnection(
     security_state::SecurityLevel level) const {
   return level == security_state::SECURE ||
          level == security_state::EV_SECURE;
 }
 
 void LocationBarViewMac::UpdateAccessibilityView(
     LocationBarDecoration* decoration) {
   if (!decoration->IsVisible())
     return;
-  NSRect r =
+  NSRect apparent_frame =
       [[field_ cell] frameForDecoration:decoration inFrame:[field_ frame]];
-  [decoration->GetAccessibilityView() setFrame:r];
-  [decoration->GetAccessibilityView() setNeedsDisplayInRect:r];
-  decoration->UpdateAccessibilityView();
+
+  // This is a bit subtle:
+  // The decorations' accessibility views can become key to allow keyboard
+  // access to the location bar decorations, but Cocoa's automatic key view loop
+  // sorts by top-left coordinate. Since the omnibox's top-left coordinate is

[lgrey, 2017/04/25 21:11:39]

Do you know if the top-left thing holds in RTL?

If the horizontal order of the loop is flipped in RTL could we do this same move
but move the "real frames" up instead of left? That way, it'd work the same for
both.

If they're *not* flipped I really have no idea what we could do short of
flipping the frames to their LTR equivalents. I'd have to think there's some
allowance, otherwise we'd have to do this for absolutely everything.

[Elly Fong-Jones, 2017/04/26 16:50:01]

It seems to proceed from the top right instead.
 
Moving them up doesn't seem to work in the LTR case - it looks like Cocoa wants
to sort views by x-then-y. I think an RTL special case might be in order here :\
Investigation reveals that the accessibility views are themselves mis-positioned
in RTL layouts. I'm not quite sure why - maybe a deficiency in
AutocompleteTextFieldCell? Per our offline discussion I changed the owner of
this TODO to you and filed https://crbug.com/715627 for this.

+  // before its leading decorations, the omnibox would sort before its own
+  // leading decorations, which was logical but visually unintuitive. Therefore,
+  // for leading decorations, this method moves their frame to be "just before"
+  // the omnibox in automatic key view loop order, and gives them an apparent
+  // frame (see DecorationAccessibilityView) so that they still paint their
+  // focus rings at the right place.
+  //
+  // TODO(ellyjones): what happens in RTL layouts?
+  NSRect real_frame = apparent_frame;
+  int left_index = [[field_ cell] leadingDecorationIndex:decoration];
+  if (left_index != -1)
+    real_frame.origin.x = [field_ frame].origin.x - left_index - 1;

[Mark Mentovai, 2017/04/25 20:44:31]

Can we have so many decorations that something’s x-coordinate might be on the
wrong side of some other nearby view?

[Elly Fong-Jones, 2017/04/26 16:50:01]

Added a DCHECK.

+
+  decoration->UpdateAccessibilityView(apparent_frame);
+  [decoration->GetAccessibilityView() setFrame:real_frame];
+  [decoration->GetAccessibilityView() setNeedsDisplayInRect:apparent_frame];
 }
 
 std::vector<LocationBarDecoration*> LocationBarViewMac::GetDecorations() {
   std::vector<LocationBarDecoration*> decorations;
   // TODO(ellyjones): content setting decorations aren't included right now, nor
   // are page actions and the keyword hint.
   decorations.push_back(location_icon_decoration_.get());
   decorations.push_back(selected_keyword_decoration_.get());
   decorations.push_back(security_state_bubble_decoration_.get());
   decorations.push_back(save_credit_card_decoration_.get());
   decorations.push_back(star_decoration_.get());
   decorations.push_back(translate_decoration_.get());
   decorations.push_back(zoom_decoration_.get());
   decorations.push_back(manage_passwords_decoration_.get());
   return decorations;
 }
 
 void LocationBarViewMac::OnDefaultZoomLevelChanged() {
   if (UpdateZoomDecoration(/*default_zoom_changed=*/true))
     OnDecorationsChanged();
 }
 
 std::vector<NSView*> LocationBarViewMac::GetDecorationAccessibilityViews() {
   std::vector<LocationBarDecoration*> decorations = GetDecorations();
   std::vector<NSView*> views;
   for (auto* decoration : decorations)
     views.push_back(decoration->GetAccessibilityView());
   return views;
 }