Clean up RenderThemeWin.
Created attachment 26872 [details] Patch
Comment on attachment 26872 [details] Patch > +Color RenderTheme::activeSelectionForegroundColor() const > +{ > + if (supportsForegroundSelectionColors() && !m_activeSelectionForegroundColor.isValid()) It would be slightly more efficient to reverse the two sides of this && (ditto elsewhere). > -Color RenderTheme::platformInactiveSelectionBackgroundColor() const > +Color RenderTheme::platformActiveSelectionForegroundColor() const > { > - // Use a grey color by default if the platform theme doesn't define anything. > - return Color(128, 128, 128); > + // Use a white color by default if the platform theme doesn't define anything. > + return Color(255, 255, 255); > } > > -Color RenderTheme::platformActiveSelectionForegroundColor() const > +Color RenderTheme::platformInactiveSelectionBackgroundColor() const > { > - return Color(); > + // Use a grey color by default if the platform theme doesn't define anything. > + return Color(176, 176, 176); Why the change from 128 to 176? > Color RenderTheme::platformInactiveSelectionForegroundColor() const > { > - return Color(); > + // Use a black color by default. > + return Color(0, 0, 0); > } I think it would be better to use Color::white, Color::black, and Color::gray instead of hand-coding the values in this file. > Color RenderThemeWin::platformInactiveSelectionBackgroundColor() const > { > - COLORREF color = GetSysColor(COLOR_GRAYTEXT); > - return Color(GetRValue(color), GetGValue(color), GetBValue(color), 255); > + return Color(176, 176, 176, 255); > } Again, I think Color::gray would be better. I don't think the fourth parameter is required. r=me
Committed as r40074. Resolving since it was committed (to move this out of the commit queue).