| Summary: | [WinCairo] Use RenderThemeAdwaita instead of RenderThemeWin | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||||||||||||
| Component: | New Bugs | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | annulen, Basuke.Suzuki, benjamin, calvaris, cdumez, changseok, cmarcelo, darin, don.olmstead, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, kondapallykalyan, pdr, philipj, ross.kirsling, ryuan.choi, sergio, webkit-bug-importer, youssefdevelops, y_soliman | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
See also: Bug 246264 – [GPU Process] Enable drawing form controls for GPU Process on macOS Created attachment 463051 [details]
Patch
Created attachment 463052 [details]
Patch
Created attachment 463064 [details]
Patch
Created attachment 463066 [details]
Patch
Comment on attachment 463066 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463066&action=review > Source/WebCore/rendering/RenderThemeAdwaita.cpp:241 > +#elif PLATFORM(WIN) The code below doesn’t look platform-specific. Can we instead just use "#else" here? Or is this actively specific to PLATFORM(WIN)? > Source/WebCore/rendering/RenderThemeAdwaita.cpp:243 > + if (!path) Typically it’s best to check isNull() instead of using ! for WTF::String. But also, maybe FileSystem::readEntireFile already harmlessly fails if passed a null string, so maybe we don’t need this check. Comment on attachment 463066 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463066&action=review Also LGTM > Source/WebCore/platform/adwaita/ScrollbarThemeAdwaita.cpp:330 > + gboolean warpSlider = FALSE; > + g_object_get(gtk_settings_get_default(), > + "gtk-primary-button-warps-slider", > + &warpSlider, nullptr); > + return warpSlider; I'm not a fan of the variable shadowing here. >> Source/WebCore/rendering/RenderThemeAdwaita.cpp:241 >> +#elif PLATFORM(WIN) > > The code below doesn’t look platform-specific. Can we instead just use "#else" here? Or is this actively specific to PLATFORM(WIN)? The webkitBundlePath code is specific to PLATFORM(WIN) at this time. Comment on attachment 463066 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=463066&action=review >> Source/WebCore/platform/adwaita/ScrollbarThemeAdwaita.cpp:330 >> + return warpSlider; > > I'm not a fan of the variable shadowing here. Will fix. >> Source/WebCore/rendering/RenderThemeAdwaita.cpp:243 >> + if (!path) > > Typically it’s best to check isNull() instead of using ! for WTF::String. But also, maybe FileSystem::readEntireFile already harmlessly fails if passed a null string, so maybe we don’t need this check. Right. Will fix. Created attachment 463071 [details]
Patch
Committed 255715@main (284e48c374b0): <https://commits.webkit.org/255715@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 463071 [details]. |
Created attachment 463020 [details] WIP patch [WinCairo] Use RenderThemeAdwaita instead of RenderThemeWin RenderThemeWin doesn't work with UseGPUProcessForDOMRenderingEnabled. Let's use RenderThemeAdwaita. Piggy backed on GTK efforts.