Show scheme in black and content in gray for data: protocol urls

components/omnibox/browser/omnibox_view.h

 // 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.
 
 // This file defines the interface class OmniboxView.  Each toolkit will
 // implement the edit view differently, so that code is inherently platform
 // specific.  However, the OmniboxEditModel needs to do some communication with
 // the view.  Since the model is shared between platforms, we need to define an
 // interface that all view implementations will share.
 
 #ifndef COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_VIEW_H_
 #define COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_VIEW_H_
 
 #include <stddef.h>
 
 #include <string>
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/gfx/range/range.h"
 
 class GURL;
 class OmniboxClient;
 class OmniboxEditController;
 class OmniboxViewMacTest;
 class OmniboxEditModel;
 
 namespace gfx {
 enum class VectorIconId;
 }
@@ skipping 216 matching lines @@
   // |GetTextLength| can't be used as the Windows override of this class
   // inherits from a class that defines a method with that name.
   virtual int GetOmniboxTextLength() const = 0;
 
   // Try to parse the current text as a URL and colorize the components.
   virtual void EmphasizeURLComponents() = 0;
 
   OmniboxEditController* controller() { return controller_; }
   const OmniboxEditController* controller() const { return controller_; }
 
+  // Subclasses use this to set colors on ranges of the text. Pass an invalid
+  // range to indicate the base level of emphasis for the text.

[Peter Kasting, 2017/03/01 02:39:11]

Nit: Maybe this would be more descriptive while being briefer:

Marks part (or, if |range| is invalid, all) of the current text as emphasized or
de-emphasized, by changing its color.

[elawrence, 2017/03/01 21:49:02]

Done.

+  virtual void SetEmphasis(bool emphasize, gfx::Range);

[Peter Kasting, 2017/03/01 02:39:11]

I'd make these two functions pure virtual.  All subclasses should have to think
about how to implement these, and if there are any today that want to do
nothing, I'd do that in those subclasses directly.

This will also help clarify that these are implemented by subclasses, which you
were trying to indicate in the function comment (and I've removed in my proposed
comment).

Nit: Name the second param

[Robert Sesek, 2017/03/01 18:29:45]

The gfx::Range argument here and on line 269 could be passed as const&.

[elawrence, 2017/03/01 21:49:01]

Done.

[elawrence, 2017/03/01 21:49:02]

Done.

+
+  // Subclasses use this to apply styles to the scheme based on the security
+  // state. An invalid range indicates that the text does not contain a scheme.
+  virtual void UpdateSchemeEmphasis(gfx::Range range);

[Peter Kasting, 2017/03/01 02:39:11]

Nit: I might call this UpdateSchemeStyle() or something, since it's not solely
about "emphasis".  You could comment it:

Sets the color and strikethrough state for |range|, which represents the current
scheme, based on the current security state.  Schemes are displayed in different
ways for different security levels.

[elawrence, 2017/03/01 21:49:01]

Done.

+
+  // Deemphasize parts of the URL to draw attention to whatever best represents

[Peter Kasting, 2017/03/01 02:39:11]

Nit: Descriptive, not imperative

[elawrence, 2017/03/01 21:49:02]

Done.

+  // the "identity" of the URL.
+  void ApplyEmphasis(const base::string16& display_text,

[Peter Kasting, 2017/03/01 02:39:11]

Nit: I wonder if this title is too specific now that this calls
UpdateSchemeEmphasis() (which I propose renaming above).  Maybe it should be
named more like UpdateTextStyle() and commented:

Parses |display_text|, then sets the display emphasis and scheme style
appropriately.  If the text is a query string, there is no scheme, and
everything is emphasized equally, whereas for URLs the scheme is styled based on
the current security state, and parts of the URL are de-emphasized to draw
attention to whatever best represents the "identity" of the current URL.  See
comments in the code for more.

[elawrence, 2017/03/01 21:49:02]

Done.

+                     const AutocompleteSchemeClassifier& classifier);
+
  private:
   friend class OmniboxViewMacTest;
 
   // |model_| can be NULL in tests.
   std::unique_ptr<OmniboxEditModel> model_;
   OmniboxEditController* controller_;
 
   DISALLOW_COPY_AND_ASSIGN(OmniboxView);
 };
 
 #endif  // COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_VIEW_H_