Bug 185854 – [GTK] Buttons are drawn too large, text not centered

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Michael Catanzaro]

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.

[Michael Catanzaro]

Created attachment 340937 [details]
launchpad.net screenshot

[Carlos Bentzen]

Created attachment 346157 [details]
Patch

[Carlos Bentzen]

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.

[Carlos Bentzen]

Created attachment 346158 [details]
launchpad.net after patch

[Carlos Bentzen]

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.

[Carlos Bentzen]

Created attachment 346160 [details]
Before and after in bugzilla button

[EWS Watchlist]

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

[EWS Watchlist]

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

[Michael Catanzaro]

Looks good to me. I think Carlos Garcia will want to review it.

[WebKit Commit Bot]

Comment on attachment 346157 [details]
Patch

Clearing flags on attachment: 346157

Committed r234592: <https://trac.webkit.org/changeset/234592>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/42961028>