New issue
Advanced search Search tips

Issue 701241 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug

Blocked on:
issue 691891



Sign in to add a comment

Bring up views::Typography "change detector" tests

Project Member Reported by tapted@chromium.org, Mar 14 2017

Issue description

Chrome Version       : 58.0.3026.3

Note these are not "Change detector" tests in the common sense - we actually want to detect changes that aren't under our control -- e.g. those that happen with OS version upgrades. 

The Harmony spec ( Issue 691891 ) relies on being able to pick a suite of gfx::Fonts with desired properties "by default".

Here, "by default" means a default OS configuration with a "typical" screen for that OS.

Users must be able to override default font sizes (and more), and we derive fonts based on a "template".

We need a mechanism to detect when the OS decides to change its default configuration, since that will affect what "templates" we must provide for that OS + version combination.

Unfortunately, not all of our trybots are in a "default" configuration, so attempting to detect this without making a flaky test is problematic.

We also need a profile of the suite of OS versions and default configurations that Chrome supports in order to determine the initial set of "templates", and this is tricky to do all at once.

So these tests will initially be landed disabled and I'll try to "smoke out" all the bots / templates with small enable/disable changes rather than trying to reland an entire test suite each time.

https://codereview.chromium.org/2734113006/
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 21 2017

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

commit 9577332120682b5400df78b16edcc0c759bb5881
Author: tapted <tapted@chromium.org>
Date: Tue Mar 21 00:51:56 2017

"Bootstrap" a toolkit-views Typography spec.

Under Harmony, we want to abstract away decisions about font selection.
Harmony has a set of styles as part of the spec that encapsulate:
- Font (typeface)
- Font size, weight and color
- Line spacing ("leading")

gfx::Font[List] can represent Font, size and weight, but not color or
line spacing. Also callers typically _never_ care about typeface, but
APIs using gfx::Font are unable to encapsulate this. One side-effect is
that a lot of callers use gfx::Font::Derive(..) rather than
ResourceBundle::GetFontWithDelta(..) and so miss out on caching
benefits.

The typography concepts are split into "TextContext" and "TextStyle".
The former describes the location of the text, and the latter describes
its current state (e.g. Disabled) or other appearance (e.g. a Hyperlink)
in that context.

This CL exposes a set of abstract typography concepts at
views::style, and explores the impact on client code by refactoring
the views::Label constructor that takes a gfx::FontList to take a
Label::CustomFont instead. Where it makes sense, callers are migrated to
typography concepts rather than use Label::CustomFont.

The transformations typically look like:
- ui::ResourceBundle::MediumFont -> views::style::CONTEXT_DIALOG_TITLE "15pt"
- GetFontListWithDelta(ui::kTitleFontSizeDelta) ->
    views::style::CONTEXT_DIALOG_TITLE
-  GetFontListWithDelta(1) -> (chrome) CONTEXT_BODY_TEXT_LARGE "13pt"
- [default constructor] or ResourceBundle::BaseFont ->
    CONTEXT_BODY_TEXT_LARGE or views::style::CONTEXT_LABEL "12pt"
- ui::ResourceBundle::SmallFont -> (chrome) CONTEXT_DEPRECATED_SMALL
    (Harmony doesn't have an 11pt font in the spec)
- ui::ResourceBundle::BoldFont -> (chrome) STYLE_EMPHASIZED
    (Harmony doesn't have a "bold" font in the spec)

These transformations, and the default TypographyProvider, are chosen to
effectively be a "no-op" so that no existing dialogs change font sizes
or other properties.

Follow-ups will:
- Implement a Harmony TypographyProvider.
- Require all Label constructors to take a typography concept or
    Label::CustomFont.
- Remove Label::SetFontList, Label::Set*Color, etc. (use typography or
    merge into CustomFont).
- Remove gfx::FontList from the API of other toolkit-views classes.
- Audit remaining users of Label::CustomFont to see if they can be
    replaced by Typography concepts.
- Remove ResourceBundle::FontStyle (SmallFont, etc.)
- Remove ui/default_style.h (ui::kFooFontSizeDelta, etc.)

BUG= 691891 , 701241

Review-Url: https://codereview.chromium.org/2734113006
Cr-Commit-Position: refs/heads/master@{#458261}

[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/chromeos/display/touch_calibrator/touch_calibrator_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/first_run/try_chrome_dialog_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/accessibility/invert_bubble_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/apps/app_info_dialog/app_info_header_panel.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/apps/app_info_dialog/app_info_permissions_panel_unittest.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/autofill/password_generation_popup_view_views.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/chrome_views_delegate.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/chrome_views_delegate.h
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/content_setting_bubble_contents.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/create_application_shortcut_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/crypto_module_password_dialog_view_unittest.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/first_run_bubble.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/frame/glass_browser_frame_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
[add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/chrome_typography.cc
[add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/chrome_typography.h
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/layout_delegate.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/layout_delegate.h
[add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/harmony/layout_delegate_unittest.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/ime/ime_warning_bubble_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/keyword_hint_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/location_bar_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/location_bar/location_icon_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/page_info/website_settings_popup_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/passwords/credentials_item_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/subtle_notification_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/sync/bubble_sync_promo_view_unittest.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/browser/ui/views/validation_message_bubble_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/chrome/test/BUILD.gn
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/app_list/views/speech_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/chromeos/ime/infolist_window.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/gfx/render_text_unittest.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/message_center/views/notifier_settings_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/BUILD.gn
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/bubble/bubble_frame_view.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/button/label_button.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/label.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/label.h
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/styled_label_unittest.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/tabbed_pane/tabbed_pane.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/controls/textfield/textfield.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/mus/aura_init.cc
[add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography.cc
[add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography.h
[add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography_provider.cc
[add] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/style/typography_provider.h
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/test/test_views_delegate.h
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/test/test_views_delegate_aura.cc
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/test/test_views_delegate_mac.mm
[modify] https://crrev.com/9577332120682b5400df78b16edcc0c759bb5881/ui/views/views_delegate.h

Components: Internals>Views

Comment 3 Deleted

Comment 4 Deleted

Sign in to add a comment