| Summary: | [GTK] Buttons are drawn too large, text not centered | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||||||||
| Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||
| Severity: | Normal | CC: | cadubentzen, cgarcia, commit-queue, ews-watchlist, mcatanzaro, otte, webkit-bug-importer | ||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||
| OS: | Linux | ||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
Created attachment 340937 [details]
launchpad.net screenshot
Created attachment 346157 [details]
Patch
The buttons were only being drawn down to the minimum size:
bool RenderThemeGadget::render(cairo_t* cr, const FloatRect& paintRect, FloatRect* contentsRect)
{
FloatRect rect = paintRect;
auto margin = marginBox();
rect.move(margin.left, margin.top);
rect.contract(margin.left + margin.right, margin.top + margin.bottom);
auto minSize = minimumSize();
rect.setWidth(std::max<float>(rect.width(), minSize.width()));
rect.setHeight(std::max<float>(rect.height(), minSize.height()));
...
}
So I created another Gadget children class RenderThemeButtonGadget, which overrides minimumSize().
Note: this is exactly like RenderThemeTextFieldGadget, but I created another class to be explicit and if further customization on buttons are needed in the future.
Created attachment 346158 [details]
launchpad.net after patch
Created attachment 346159 [details]
bugs.webkit.org after patch
In bugs.webkit.org it is more subtle to notice the patch, because it still is not completely aligned with the text entries.
One way to do so is use the inspector to see the margins drawn before and after. The background was being drawn outside the borders and now it is not.
Created attachment 346160 [details]
Before and after in bugzilla button
Comment on attachment 346157 [details] Patch Attachment 346157 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8712147 New failing tests: http/tests/security/video-poster-cross-origin-crash2.html imported/blink/transitions/unprefixed-transform.html legacy-animation-engine/imported/blink/transitions/unprefixed-transform.html Created attachment 346187 [details]
Archive of layout-test-results from ews206 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Looks good to me. I think Carlos Garcia will want to review it. Comment on attachment 346157 [details] Patch Clearing flags on attachment: 346157 Committed r234592: <https://trac.webkit.org/changeset/234592> All reviewed patches have been landed. Closing bug. |
Created attachment 340936 [details] bugs.webkit.org screenshot Look at these attached screenshots from bugs.webkit.org and launchpad.net. Then buttons are drawn too large. There's too much space on the buttom: the text is not vertically centered. On the bugs.webkit.org screenshot, the Search button at the top demonstrates the problem: the button is supposed to have the same vertical height as the search entry, and it's placed correctly at the top, but it uses too much vertical space on the bottom. I think the Quick Search button in the middle has the same problem, but it's much more subtle and less obvious here. It's most obvious with the Search button on the bottom, notice how it's not aligned at all with the dotted lines. On launchpad.net, the problem is pretty obvious with the "Log Out" and "Search Launchpad" buttons.