BrowserNonClientFrameViewMac::PaintThemedFrame() implementation incorrect |
||||||
Issue descriptionBrowserNonClientFrameViewMac::PaintThemedFrame() is simplistic. It tiles the frame image at 0,0 with default flags, then draws the overlay over it. Compare to GlassBrowserFrameView::PaintTitlebar(). It draws a solid color first, then tiles the frame image using vertical mirroring, ensuring that the image always starts ThemeProperties::kFrameHeightAboveTabs px above the top of the tabs. Finally it draws the overlay image. So at least those three things -- the background color, the mirroring flag, and the y offset -- need to be implemented. I consider this reasonably high-priority so custom themes work on Mac like they do on other platforms.
,
Nov 14
***UI Mass Triage*** Adding appropriate label for expert review.
,
Nov 15
To dfried@ for triage.
,
Dec 7
Do we have an example of this code actually causing visual artifacts in the frame rendering on Mac? Do we have test cases? If not it's really hard for me to prioritize this work. If this is just Mac-specific code cleanup, I'd prefer that either PK or Elly owned the issue and prioritized it accordingly.
,
Dec 7
Yes, the frame background image is painted at the wrong offset and with the wrong tiling on Mac, for all custom themes that use background images. Not cleanup, this is a functional bug. You can use this theme as a testcase: https://chrome.google.com/webstore/detail/hedgehog-in-the-fog/haocganpkafanhkfldbbmhcpaelmkejg?hl=en
,
Dec 13
Assigning per #4.
,
Dec 13
Mac triage: to lgrey@ for M73.
,
Dec 13
Add more tabs; as background tabs and the new tab button overlay the frame they should appear as if they're semitransparent, but I think you'll actually get a 7 px vertical misalignment, which I'd expect to be visible where those dark shapes are. Basically you want BrowserNonClientFrameViewMac::PaintThemedFrame() to mostly copy the code from GlassBrowserFrameView::PaintTitlebar(). The theme I linked isn't a good test case for the background color and mirroring stuff, just the y offset. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ellyjo...@chromium.org
, Aug 28