Bug 199377

Summary:

[WPE] Draw other button types

Product:

WebKit

Reporter:

Joshua Watt <JPEW.hacker>

Component:

WPE WebKit

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bugs-noreply, cgarcia, commit-queue, mcatanzaro

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Joshua Watt

Reported 2019-07-01 13:59:13 PDT

[WPE] Draw other button types

Attachments
Patch (2.12 KB, patch) 2019-07-01 13:59 PDT, Joshua Watt	no flags	Details Formatted Diff Diff
Patch (2.09 KB, patch) 2019-07-16 14:05 PDT, Joshua Watt	no flags	Details Formatted Diff Diff
Patch (2.91 KB, patch) 2019-07-24 11:37 PDT, Joshua Watt	no flags	Details Formatted Diff Diff
Patch (2.87 KB, patch) 2019-07-25 08:23 PDT, Joshua Watt	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Joshua Watt

Comment 1 2019-07-01 13:59:28 PDT

Created attachment 373259 [details] Patch

Michael Catanzaro

Comment 2 2019-07-16 07:59:48 PDT

Comment on attachment 373259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373259&action=review Could you please add a ChangeLog entry (Tools/Scripts/prepare-ChangeLog -b 199377)? Then the ChangeLog can explain more directly why you're changing what you're changing here. > Source/WebCore/platform/wpe/ThemeWPE.cpp:71 > + case DefaultButtonPart: > + case ButtonPart: Please use fallthrough annotations: case DefaultButtonPart: FALLTHROUGH; case ButtonPart: FALLTHROUGH; case SquareButtonPart: > Source/WebCore/platform/wpe/ThemeWPE.cpp:142 > + float roundness = 2; > + > + if (part == SquareButtonPart) > + roundness = 0; It'd be nicer to write this in one line IMO: float roundness = (part == SquareButtonPart) ? 0 : 2;

Carlos Garcia Campos

Comment 3 2019-07-16 08:10:43 PDT

Comment on attachment 373259 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=373259&action=review >> Source/WebCore/platform/wpe/ThemeWPE.cpp:71 >> + case ButtonPart: > > Please use fallthrough annotations: > > case DefaultButtonPart: > FALLTHROUGH; > case ButtonPart: > FALLTHROUGH; > case SquareButtonPart: I don't think we want FALLTHROUGH in those cases, because it's obvious. FALLTHROUGH is useful when you actually have code in the case and want to fallthrough to the next case, to make it explicit that you haven't forgotten the break.

Michael Catanzaro

Comment 4 2019-07-16 08:40:35 PDT

OK, as you prefer

Joshua Watt

Comment 5 2019-07-16 14:05:53 PDT

Created attachment 374239 [details] Patch

Joshua Watt

Comment 6 2019-07-16 14:07:03 PDT

Sorry I forgot the changelog. This is my first WebKit bug, the tooling scripts for uploading changes are still a little clunky to me.

Michael Catanzaro

Comment 7 2019-07-16 16:25:59 PDT

(In reply to Joshua Watt from comment #5) > Created attachment 374239 [details] > Patch This new patch is actually still missing the ChangeLog. :)

Joshua Watt

Comment 8 2019-07-24 11:37:59 PDT

Created attachment 374790 [details] Patch

Michael Catanzaro

Comment 9 2019-07-24 12:04:16 PDT

Comment on attachment 374790 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374790&action=review > Source/WebCore/ChangeLog:8 > + [WPE] Draw other button types Don't duplicate the title here.

Joshua Watt

Comment 10 2019-07-25 08:23:16 PDT

Created attachment 374889 [details] Patch

Michael Catanzaro

Comment 11 2019-07-25 09:37:44 PDT

Comment on attachment 374889 [details] Patch Request cq? when you're ready for commit.

WebKit Commit Bot

Comment 12 2019-07-25 11:04:22 PDT

Comment on attachment 374889 [details] Patch Clearing flags on attachment: 374889 Committed r247827: <https://trac.webkit.org/changeset/247827>

WebKit Commit Bot

Comment 13 2019-07-25 11:04:24 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.