Bug 146320

Summary:

Stop using ScrollbarThemeClient and just use Scrollbar directly

Product:

WebKit

Reporter:

Anders Carlsson <andersca>

Component:

New Bugs

Assignee:

Anders Carlsson <andersca>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, commit-queue, ossy

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	thorton: review+

Anders Carlsson

Reported 2015-06-25 12:41:23 PDT

Stop using ScrollbarThemeClient and just use Scrollbar directly

Attachments
Patch (143.85 KB, patch) 2015-06-25 12:49 PDT, Anders Carlsson	no flags	Details Formatted Diff Diff
Patch (142.74 KB, patch) 2015-06-25 13:01 PDT, Anders Carlsson	thorton: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Anders Carlsson

Comment 1 2015-06-25 12:49:43 PDT

Created attachment 255567 [details] Patch

WebKit Commit Bot

Comment 2 2015-06-25 12:51:47 PDT

Attachment 255567 [details] did not pass style-queue: ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:110: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:111: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:114: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:110: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:111: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:117: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:118: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:117: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:118: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:279: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:280: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:281: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:293: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:294: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:293: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:294: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:152: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:158: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/Scrollbar.cpp:342: Wrong number of spaces before statement. (expected: 26) [whitespace/indent] [4] ERROR: Source/WebCore/platform/Scrollbar.cpp:350: Wrong number of spaces before statement. (expected: 26) [whitespace/indent] [4] ERROR: Source/WebCore/platform/Scrollbar.cpp:342: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/Scrollbar.cpp:410: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:149: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:151: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:153: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:234: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:234: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:235: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:290: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:291: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 30 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.

Anders Carlsson

Comment 3 2015-06-25 13:01:25 PDT

Created attachment 255568 [details] Patch

WebKit Commit Bot

Comment 4 2015-06-25 13:05:02 PDT

Attachment 255568 [details] did not pass style-queue: ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:110: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:111: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:114: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:110: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:111: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:117: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:118: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:117: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeSafari.cpp:118: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:279: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:280: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:281: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:293: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:294: Wrong number of spaces before statement. (expected: 17) [whitespace/indent] [4] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:293: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/mac/ScrollbarThemeMac.mm:294: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:152: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:158: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/Scrollbar.cpp:342: Wrong number of spaces before statement. (expected: 26) [whitespace/indent] [4] ERROR: Source/WebCore/platform/Scrollbar.cpp:350: Wrong number of spaces before statement. (expected: 26) [whitespace/indent] [4] ERROR: Source/WebCore/platform/Scrollbar.cpp:342: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/Scrollbar.cpp:410: An else if statement should be written as an if statement when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:149: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:151: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:153: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:234: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:234: Multi line control clauses should use braces. [whitespace/braces] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:235: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:290: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] ERROR: Source/WebCore/platform/win/ScrollbarThemeWin.cpp:291: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 30 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.

Anders Carlsson

Comment 5 2015-06-25 15:04:15 PDT

Committed r185964: <http://trac.webkit.org/changeset/185964>

Alex Christensen

Comment 6 2015-06-25 18:03:53 PDT

This broke the windows build.

Csaba Osztrogonác

Comment 7 2015-06-25 23:46:04 PDT

(In reply to comment #6) > This broke the windows build. Brent landed the fix in https://trac.webkit.org/changeset/185975. One more reason why we need a working Windows EWS. But we don't have it 2 weeks ago. It seems solving a network issue needs at least a months for Apple. :)

Anders Carlsson

Comment 8 2015-06-26 09:44:23 PDT

(In reply to comment #7) > But we don't have it 2 weeks ago. It seems solving > a network issue needs at least a months for Apple. :) That's so much for your always helpful comments, Csaba.

Csaba Osztrogonác

Comment 9 2015-06-26 09:50:22 PDT

(In reply to comment #8) > (In reply to comment #7) > > > But we don't have it 2 weeks ago. It seems solving > > a network issue needs at least a months for Apple. :) > > That's so much for your always helpful comments, Csaba. I didn't want to add a helpful comment here, but a strong note. It's simply ridiculous that 2 weeks weren't enough to make developer.apple.com available for Apple Windows EWS bots ...

Anders Carlsson

Comment 10 2015-06-26 11:31:29 PDT

(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > > > But we don't have it 2 weeks ago. It seems solving > > > a network issue needs at least a months for Apple. :) > > > > That's so much for your always helpful comments, Csaba. > > I didn't want to add a helpful comment here, but a strong note. > It's simply ridiculous that 2 weeks weren't enough to make > developer.apple.com available for Apple Windows EWS bots ... Don't you think it's better to bring that up on one of our mailing lists instead of in a random bugzilla bug?

Csaba Osztrogonác

Comment 11 2015-06-26 11:38:19 PDT

(In reply to comment #10) > Don't you think it's better to bring that up on one of our mailing lists > instead of in a random bugzilla bug? I did write private mails to the bot maintainers twice in the latest two weeks, but nothing happened only a promise that it will be fixed in the (far) future. What kind of random bug do you talk? I commented this bug, because this patch broke the Windows build because of the lack of working Windows EWS.

Anders Carlsson

Comment 12 2015-06-26 11:42:15 PDT

(In reply to comment #11) > (In reply to comment #10) > > Don't you think it's better to bring that up on one of our mailing lists > > instead of in a random bugzilla bug? > > I did write private mails to the bot maintainers twice in the latest two > weeks, > but nothing happened only a promise that it will be fixed in the (far) > future. > > What kind of random bug do you talk? I commented this bug, because this > patch broke the Windows build because of the lack of working Windows EWS. And you're just wasting your (as well as my) time because I can't do anything about it.

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