Summary: | [chromium] Update WebScrollbar so that it works with overlay scrollbars on Lion | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | John Abd-El-Malek <jam> | ||||||||||||
Component: | New Bugs | Assignee: | John Abd-El-Malek <jam> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | dglazkov, fishd, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
John Abd-El-Malek
2011-08-10 13:20:50 PDT
Created attachment 103526 [details]
Patch
This is the patch you reviewed in http://codereview.chromium.org/7538006/. There are some style errors because I've separated the chromium and non-chromium headers in the cpp files. This seemed to be the style of the newer chrome WebKit files, so I've followed it, although I don't care either way. also, I can't roll this until the chromium side patch lands. So as soon as that does, I'll update this with a DEPS roll for chrome Attachment 103526 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1
Source/WebKit/chromium/src/ScrollbarGroup.cpp:37: Alphabetical sorting problem. [build/include_order] [4]
Source/WebKit/chromium/src/WebScrollbarImpl.cpp:44: Alphabetical sorting problem. [build/include_order] [4]
Source/WebKit/chromium/src/WebScrollbarImpl.cpp:47: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 3 in 11 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 103527 [details]
Patch
Attachment 103527 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/chromium/ChangeLog', u'Sourc..." exit_code: 1
Source/WebKit/chromium/src/ScrollbarGroup.cpp:37: Alphabetical sorting problem. [build/include_order] [4]
Source/WebKit/chromium/src/WebScrollbarImpl.cpp:44: Alphabetical sorting problem. [build/include_order] [4]
Source/WebKit/chromium/src/WebScrollbarImpl.cpp:47: Alphabetical sorting problem. [build/include_order] [4]
Total errors found: 3 in 12 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 103527 [details] Patch Attachment 103527 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9339771 Created attachment 103549 [details]
Patch
Created attachment 103552 [details]
Patch
Created attachment 103554 [details]
Patch
(latest patch just fixed spacing) Darin: please have a look Comment on attachment 103554 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103554&action=review R=me > Source/WebKit/chromium/src/ScrollbarGroup.cpp:8 > + * * Redistributions of source code must retain the above copyright fyi, this is the wrong copyright header format. we're supposed to use the 2-clause version for new stuff (like in http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp, for instance) > Source/WebKit/chromium/src/ScrollbarGroup.cpp:46 > + : m_page(page), > + m_horizontalScrollbar(0) nit: comma goes on the next line > Source/WebKit/chromium/src/ScrollbarGroup.cpp:79 > + } else { > + willRemoveVerticalScrollbar(scrollbar->scrollbar()); might as well ASSERT() this is the vertical scrollbar > Source/WebKit/chromium/src/ScrollbarGroup.h:48 > + ScrollbarGroup(WebCore::Page*); add 'explicit' Committed r92811: <http://trac.webkit.org/changeset/92811> |