Bug 66504 – Chromium Mac: Need tests for switching between overlay and opaque scrollbars

• Component: Platform
• Status: RESOLVED WONTFIX

[Sailesh Agrawal]

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.

[Sailesh Agrawal]

Created attachment 105245 [details]
Patch

[Sailesh Agrawal]

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 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]

(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 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 on attachment 105245 [details]
Patch

Attachment 105245 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9513919

[Beth Dakin]

(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 on attachment 105245 [details]
Patch

r- based on bdakin's comments