RESOLVED WONTFIX 66504
Chromium Mac: Need tests for switching between overlay and opaque scrollbars 
https://bugs.webkit.org/show_bug.cgi?id=66504
Summary Chromium Mac: Need tests for switching between overlay and opaque scrollbars
Sailesh Agrawal
Reported 2011-08-18 15:40:22 PDT
This bug is to track the work need to add new tests to test the scrollbar drawing code when switching between opaque and overlay scrollbars.
Attachments
Patch (52.07 KB, patch)
2011-08-25 14:35 PDT, Sailesh Agrawal 		thakis: review-
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Sailesh Agrawal
Comment 1 2011-08-25 14:35:41 PDT
Created attachment 105245 [details] Patch
Sailesh Agrawal
Comment 2 2011-08-25 14:39:53 PDT
This is not ready to be checked in but I wanted to send it out to get some early feedback. dglazkov - is adding the new window.internals API ok? bdakin - does this approach seem ok? if so do you want me to make similar changes to ScrollbarThemeMac and ScrollAnimatorMac? jamesr / thakis: I'm not sure where the test files are supposed to go. Does this seem correct? The tests in this patch don't actually change the scrollbar type at run time. Once this is checked in I'll send out a separate patch with that test.
Dimitri Glazkov (Google)
Comment 3 2011-08-25 14:53:13 PDT
Comment on attachment 105245 [details] Patch Is window.internals the right spot to expose this API? The code looks pretty chromium-specific, so perhaps this should be a layoutTestController API?
Sailesh Agrawal
Comment 4 2011-08-25 14:54:44 PDT
(In reply to comment #3) > (From update of attachment 105245 [details]) > Is window.internals the right spot to expose this API? The code looks pretty chromium-specific, so perhaps this should be a layoutTestController API? This is actually mac specific. It should work Safari's mac port as well.
WebKit Review Bot
Comment 5 2011-08-25 17:29:51 PDT
Comment on attachment 105245 [details] Patch Attachment 105245 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9507678 New failing tests: platform/mac-lion/scrollbars/opaque-scrollbar.html platform/mac-lion/scrollbars/overlay-scrollbar.html
Collabora GTK+ EWS bot
Comment 6 2011-08-25 22:35:02 PDT
Comment on attachment 105245 [details] Patch Attachment 105245 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9513919
Beth Dakin
Comment 7 2011-08-26 15:51:16 PDT
(In reply to comment #2) > bdakin - does this approach seem ok? if so do you want me to make similar changes to ScrollbarThemeMac and ScrollAnimatorMac? Hi Sailesh, this looks like a good approach! It would be awesome if you would add similar code to ScrollbarThemeMac and ScrollAnimatorMac. My one comment about the naming in the ScrollbarStyle enum, which currently looks like this in your patch: enum ScrollbarStyle { ScrollbarStyleSystemDefault, ScrollbarStyleLegacy, ScrollbarStyleOverlay }; The phrase "legacy scrollbar" is meaningful on the Mac platform, but it's not meaningful on other platforms. Since this enum lives and will be used in cross-platform code, I recommend a name that makes sense on all platforms. What about "physical scrollbars," or specifically ScrollbarStylePhysical for the enum?
Nico Weber
Comment 8 2011-09-16 16:08:28 PDT
Comment on attachment 105245 [details] Patch r- based on bdakin's comments
Note You need to log in before you can comment on or make changes to this bug.