When you click in the scrollbar trough, WebKit GTK will always warp the slider to the clicked position, regardless of the setting 'gtk-primary-button-warps-slider'. Expected behaviour: If 'gtk-primary-button-warps-slider' is false, then a click in the trough should page forward or backward; if true, then it should warp. Holding down the shift key or clicking the middle button reverses the sense of the setting. Reference: https://developer.gnome.org/gtk3/stable/GtkSettings.html#GtkSettings--gtk-primary-button-warps-slider
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>