| Summary: | Custom scrollbars should not create ScrollbarPainters on Mac | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | bdakin, bunhere, cdumez, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, kondapallykalyan, sam, sergio, simon.fraser, thorton | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Beth Dakin
2014-06-27 12:06:32 PDT
Created attachment 234005 [details]
Patch
Attachment 234005 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/Scrollbar.cpp:59: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/WebCore/platform/Scrollbar.cpp:82: Wrong number of spaces before statement. (expected: 25) [whitespace/indent] [4]
Total errors found: 2 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Thanks Tim http://trac.webkit.org/changeset/170546 Comment on attachment 234005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234005&action=review > Source/WebCore/platform/Scrollbar.h:201 > + bool m_isCustomScrollbar; Why protected instead of private? > Source/WebCore/rendering/RenderScrollbar.cpp:45 > + : Scrollbar(scrollableArea, orientation, RegularScrollbar, RenderScrollbarTheme::renderScrollbarTheme(), true) Makes me think we should use an enum instead of a bool for readability. (In reply to comment #4) > (From update of attachment 234005 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234005&action=review > > > Source/WebCore/platform/Scrollbar.h:201 > > + bool m_isCustomScrollbar; > > Why protected instead of private? > Oh, that was very silly. This should definitely be private, I just failed to notice that all of the other member variables were protected when I put this one next to them. > > Source/WebCore/rendering/RenderScrollbar.cpp:45 > > + : Scrollbar(scrollableArea, orientation, RegularScrollbar, RenderScrollbarTheme::renderScrollbarTheme(), true) > > Makes me think we should use an enum instead of a bool for readability. That would definitely look nicer. I'll do a follow-up cleanup here. |