[WPE] checkbox and radio button aren't painted because ThemeAdwaita::m_accentColor is the invalid color
Created attachment 463046 [details] Patch
Comment on attachment 463046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463046&action=review > Source/WebCore/platform/adwaita/ThemeAdwaita.cpp:-540 > - return m_accentColor.isValid() ? m_accentColor : SRGBA<uint8_t> { 52, 132, 228 }; Is this part needed given it's about the default color? Though really might be better to set it externally, like GTK port does.
Comment on attachment 463046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463046&action=review >> Source/WebCore/platform/adwaita/ThemeAdwaita.cpp:-540 >> - return m_accentColor.isValid() ? m_accentColor : SRGBA<uint8_t> { 52, 132, 228 }; > > Is this part needed given it's about the default color? Though really might be better to set it externally, like GTK port does. Sorry, but I don't understand your suggestion. GTK port is using gtk_style_context_lookup_color to the system accent color. However, WPE can't do that. I think it's a good idea to have the default accent color here.
I know it can't do that, but it can set the hardcoded default color from the same place. E.g. if WPE port adds API to set the accent color at some point, it would make it easier. Though that's not important, my main question is why do you need to both set the default color and check color validity later - shouldn't just one of those be enough here?
(In reply to Alexander Mikhaylenko from comment #4) > I know it can't do that, but it can set the hardcoded default color from the > same place. E.g. if WPE port adds API to set the accent color at some point, > it would make it easier. Yup, it seems a good idea to add a new API for WPE port. I'm going to use RenderThemeAdwaita for WinCairo port now (bug#246604). I think here is the good place to have the default accent color for ports without HAVE_APP_ACCENT_COLORS. > Though that's not important, my main question is why do you need to both set > the default color and check color validity later - shouldn't just one of > those be enough here? My patch actually removes the validity checking.
I misunderstood your comment. paintCheckbox doesn't check the validity. https://github.com/WebKit/WebKit/blob/d9124122cd00f55a24eb658fe1e12b4a47ff9513/Source/WebCore/platform/adwaita/ThemeAdwaita.cpp#L262
Oh whoops, I shouldn't review patches at 6 AM. LGTM 😅️
> I misunderstood your comment. No, you didn't. I indeed misread the patch as adding that check instead of removing it.
Committed 255668@main (d03dce63e768): <https://commits.webkit.org/255668@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 463046 [details].