Summary: | [GTK] Scrollbars not following gtk-primary-button-warps-slider setting | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jim Mason <jmason> | ||||||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | bugs-noreply, cgarcia, commit-queue, ews-watchlist, mcatanzaro | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Attachments: |
|
Description
Jim Mason
2018-10-30 07:47:25 PDT
Created attachment 353366 [details]
Patch
Attachment 353366 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:780: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:781: Use nullptr instead of NULL. [readability/null] [5]
ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:781: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:783: 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/gtk/ScrollbarThemeGtk.cpp:784: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
Total errors found: 5 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 353367 [details]
Patch
Attachment 353367 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:780: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3]
ERROR: Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:784: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3]
Total errors found: 2 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 353369 [details]
Patch
Did you close this accidentally? Comment on attachment 353369 [details]
Patch
Oooh, cool! Nice, especially if this is your first contribution.
I have two suggestions:
(1) WebKit style is that the comment should be a complete sentence that starts with a capital and ends with a period.
(2) The ^ is too clever for my taste; can you write it out with conditionals so that it's easier to read?
Thanks!
Created attachment 353387 [details]
Patch
(In reply to Michael Catanzaro from comment #7) > Comment on attachment 353369 [details] > Patch > > Oooh, cool! Nice, especially if this is your first contribution. > > I have two suggestions: > > (1) WebKit style is that the comment should be a complete sentence that > starts with a capital and ends with a period. > > (2) The ^ is too clever for my taste; can you write it out with > conditionals so that it's easier to read? > > Thanks! Good feedback, thanks! The changes have been applied. Comment on attachment 353387 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353387&action=review > Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:785 > + return warp_slider? OK one last change: leave one space before the ? warp_slider ? Sorry. :P Created attachment 353389 [details]
Patch
(In reply to Michael Catanzaro from comment #10) > Comment on attachment 353387 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=353387&action=review > > > Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:785 > > + return warp_slider? > > OK one last change: leave one space before the ? > > warp_slider ? > > Sorry. :P No prob, 'tis done. Comment on attachment 353389 [details] Patch Clearing flags on attachment: 353389 Committed r237617: <https://trac.webkit.org/changeset/237617> All reviewed patches have been landed. Closing bug. (In reply to Michael Catanzaro from comment #10) > Comment on attachment 353387 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=353387&action=review > > > Source/WebCore/platform/gtk/ScrollbarThemeGtk.cpp:785 > > + return warp_slider? > > OK one last change: leave one space before the ? > > warp_slider ? > > Sorry. :P warpSlider... Sorry, I missed that. Will you rename it, or do you want me to? Reopening to attach new patch. Created attachment 355518 [details]
Patch
Attachment 355518 [details] did not pass style-queue:
ERROR: Source/WebCore/ChangeLog:10: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 355519 [details]
Patch
Comment on attachment 355519 [details]
Patch
OK, thanks.
Comment on attachment 355519 [details] Patch Clearing flags on attachment: 355519 Committed r238462: <https://trac.webkit.org/changeset/238462> All reviewed patches have been landed. Closing bug. |