Need to reduce the number of places we ask for Windows system fonts. |
||||||
Issue description
We call base::win::GetNonClientMetrics() in several places in the code to get the default Windows fonts for menus, captions, etc.
There are a problems with this approach:
- Every time we call this function to get fonts, we then have to call:
- display::win::AdjustFontForAccessibility(...)
- l10n_util::AdjustUIFont(...)
- We need to do these steps each time, and for each affected font.
- This leads to both:
- Fragile code duplication around the codebase.
- Leaking of Windows API-specific structs in places where we don't actually
care about the API-level specifics.
I want to hide NONCLIENTMETRICS_XP and specifically LOGFONT* structs from code outside of platform_font_win.cc when dealing with anything around getting Windows system fonts.
I want to have code in platform_font_win.cc be responsible for doing all font adjustments prior to reporting them to other Chrome code (e.g. by calling AdjustFontForAccessibility() and AdjustUIFont() internally).
OPTIONALLY: I might want to make sure that all font information is loaded at the same time so that changing a system font while Chrome is running doesn't result in inconsistent fonts between one part of the browser and another. (This could happen with the main menu, which re-loads its fonts when it's shown.)
,
Sep 26
,
Sep 27
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aab1a7e5171e1b9cab6f97047f823fa585282481 commit aab1a7e5171e1b9cab6f97047f823fa585282481 Author: Dana Fried <dfried@chromium.org> Date: Thu Sep 27 22:04:02 2018 Consolidate fetching Windows system fonts into platform_font_win. Half of requested cleanup following text scaling issues with Windows Text Zoom accessibility feature. Removes calls to GetNonClientMetrics() sprinkled around the code, as well as most uses of the Windows LOGFONT struct outside of platform_font_win and its callbacks. The remainder of the cleanup will be in a followup CL, which will consolidate all of the fiddly scaling and other uses of LOGFONT into platform_font_win. Bug: 888788 Change-Id: Iff47d2037b6a70210bd1eb0f24ab2f0430c59f86 Reviewed-on: https://chromium-review.googlesource.com/1246824 Reviewed-by: Robert Liao <robliao@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/heads/master@{#594889} [modify] https://crrev.com/aab1a7e5171e1b9cab6f97047f823fa585282481/chrome/browser/chrome_browser_main_win.cc [modify] https://crrev.com/aab1a7e5171e1b9cab6f97047f823fa585282481/content/browser/renderer_host/render_view_host_impl.cc [modify] https://crrev.com/aab1a7e5171e1b9cab6f97047f823fa585282481/ui/gfx/font_unittest.cc [modify] https://crrev.com/aab1a7e5171e1b9cab6f97047f823fa585282481/ui/gfx/platform_font_win.cc [modify] https://crrev.com/aab1a7e5171e1b9cab6f97047f823fa585282481/ui/gfx/platform_font_win.h [modify] https://crrev.com/aab1a7e5171e1b9cab6f97047f823fa585282481/ui/views/controls/menu/menu_config_win.cc [modify] https://crrev.com/aab1a7e5171e1b9cab6f97047f823fa585282481/ui/views/widget/native_widget_aura.cc
,
Sep 28
,
Sep 28
,
Oct 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8abd68396d3599abe18fa5797ec6dff2f8f68fad commit 8abd68396d3599abe18fa5797ec6dff2f8f68fad Author: Dana Fried <dfried@chromium.org> Date: Wed Oct 03 17:39:17 2018 Consolidate font adjustment into PlatformFontWin. Also fixes a bug where tooltips in Hindi and other languages that use custom font sizes would revert when the window DPI changes. Bug: 888788 Change-Id: I490a425d7bf142ef385462837680dfb34f5ff544 Reviewed-on: https://chromium-review.googlesource.com/1250211 Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Robert Liao <robliao@chromium.org> Commit-Queue: Dana Fried <dfried@chromium.org> Cr-Commit-Position: refs/heads/master@{#596275} [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/chrome/browser/chrome_browser_main_win.cc [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/base/BUILD.gn [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/base/l10n/l10n_util_win.cc [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/base/l10n/l10n_util_win.h [delete] https://crrev.com/2b798e975c6cdb02dee93bdaf184276ad21d4be9/ui/base/l10n/l10n_util_win_unittest.cc [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/display/win/dpi.cc [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/display/win/dpi.h [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/gfx/platform_font_win.cc [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/gfx/platform_font_win.h [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/gfx/platform_font_win_unittest.cc [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/views/corewm/tooltip_win.cc [modify] https://crrev.com/8abd68396d3599abe18fa5797ec6dff2f8f68fad/ui/views/corewm/tooltip_win.h
,
Oct 4
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by dfried@chromium.org
, Sep 26