904561 - [10.7] Scrollbars shouldn't grow on hover on OSX 10.7 (only on 10.8 and up)

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Bogdan Maris, Desktop Test Engineering]

Reproducible on the Firefox 24 Beta 2 (BuildID: 20130812173056):
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:24.0) Gecko/20100101 Firefox/24.0
Reproducible on the latest Aurora (BuildID: 20130812004004):
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:25.0) Gecko/20100101 Firefox/25.0
Reproducible on the latest Nightly (BuildID: 20130812030209): 
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:26.0) Gecko/20100101 Firefox/26.0

Steps to reproduce:
1. Start Firefox.
2. Open a page where you can scroll.
3. Scroll using the mouse.
4. Move the cursor to the scrollbar when it`s displayed.

Expected results: The scrollbar is properly displayed.

Actual results: The scrollbar moves to the left.

Comment 1

[Stephen A Pohl [:spohl]]

Bogdan, are you referring to the fact that the scrollbar is 'growing', i.e. becoming wider when hovered? Is what you're seeing different from Safari? Screenshots would be appreciated. Thanks!

Comment 2

[Bogdan Maris, Desktop Test Engineering]

Attachment: Screencast showing the issue

I posted a screencast that will show the issue. It will need Quicktime software to play it. Note that this behavior is specific only for Firefox. Chrome, Safari and Opera does not show this issue.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

I notice from Bogdan's video that the scrollbar does actually move (left a few pixels), but only some of the time.

Comment 4

[Markus Stange [:mstange]]

Oh, right, on 10.7 scrollbars didn't actually grow! We need to limit the size change to 10.8 and above.

Comment 5

[Stephen A Pohl [:spohl]]

Assigning to myself. Need to install 10.7 to verify my patch and check against native apps.

Comment 7

[Stephen A Pohl [:spohl]]

Markus, I've been trying to figure out how you've determined the parameters that are passed to CUIDraw for our scrollbars when hovered. I can successfully set conditional breakpoints in gdb that are hit when the @"widget" key in the dictionary has value @"kCUIWidgetOverlayScrollBar". However, I'm unable to figure out how to make the breakpoint conditional on the mouse hovering over the scrollbar. I was thinking I could write gdb breakpoint command lists to print out the contents of the NSDictionary in $edx and continue without breaking (essentially a form of printf inside CUIDraw, rather than a breakpoint). But I've been unable to figure out how to set local variables in command lists (maybe not supported?). As my next step, I was going to use an interpose library to print the contents of the NSDictionary, but I wanted to check with you if there's an easier way. You must have figured out somehow that @"state" had value of @"rollover" on OSX 10.8 when the scrollbar is hovered, and I'm basically curious to hear how you've figured that out.

Comment 8

[Markus Stange [:mstange]]

(In reply to Stephen Pohl [:spohl] from comment #7)
> However, I'm unable to figure out how to make the breakpoint conditional on
> the mouse hovering over the scrollbar.

I didn't do anything that sophisticated. I just kept doing "c" and "po $rdx" until I hit the call for scrollbar drawing.

It's easier to hit it if you keep the gdb window focused and only enable the breakpoint if you've hovered the scrollbar in the window you're targeting (which stays in the background). Like this, for example: http://tests.themasta.com/gdbcuidrawoverlayscrollbar.webm

Comment 9

[Stephen A Pohl [:spohl]]

Attachment: Patch

Comment 10

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 793589 [details] [diff] [review]
Patch

This basically looks fine to me, though I haven't tested it.

Are we using just one style for scrollbars (one appropriate for the hovered state on 10.8 and up), and adjusting it when it isn't hovered (or we're on 10.7 and below)?

+        if (!nsCocoaFeatures::OnMountainLionOrLater()) {
+          if (isHorizontal) {
+            macRect.origin.y += 2.0;
+          } else {
+            macRect.origin.x += 3.0;
+          }
+        }

Where do these numbers come from?  Trial and error?

Comment 11

[Stephen A Pohl [:spohl]]

(In reply to Steven Michaud from comment #10)
> Are we using just one style for scrollbars (one appropriate for the hovered
> state on 10.8 and up), and adjusting it when it isn't hovered (or we're on
> 10.7 and below)?

That's correct. We wouldn't get here on OSX below version 10.7 though, because nsLookAndFeel::UseOverlayScrollbars() would already be false (since the features is for 10.7+).

> 
> +        if (!nsCocoaFeatures::OnMountainLionOrLater()) {
> +          if (isHorizontal) {
> +            macRect.origin.y += 2.0;
> +          } else {
> +            macRect.origin.x += 3.0;
> +          }
> +        }
> 
> Where do these numbers come from?  Trial and error?

Yes, trial and error. :-(

Comment 12

[Stephen A Pohl [:spohl]]

Attachment: Patch

Added comment that numbers were obtained by trial and error, as requested on IRC. Carrying over r+.

Comment 13

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/58d1b62f613c

Comment 14

[Stephen A Pohl [:spohl]]

Attachment: Followup for RTL support

Forgot about RTL before.

Comment 15

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/67775ddf0d15

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/58d1b62f613c
https://hg.mozilla.org/mozilla-central/rev/67775ddf0d15

Comment 17

[Stephen A Pohl [:spohl]]

Comment on attachment 793637 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 636564
User impact if declined: Overlay scrollbars on OSX 10.7 would grow when hovered, which doesn't match the native behavior.
Testing completed (on m-c, etc.): Has been on m-c for ~1 day. Local testing shows that the issue is fixed.
Risk to taking this patch (and alternatives if risky): very low
String or IDL/UUID changes made by this patch: none

Comment 18

[Stephen A Pohl [:spohl]]

Comment on attachment 793653 [details] [diff] [review]
Followup for RTL support

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 636564
User impact if declined: Overlay scrollbars on OSX 10.7 in RTL mode would display incorrectly when hovered.
Testing completed (on m-c, etc.): Has been on m-c for ~1 day. Local testing shows that the issue is fixed.
Risk to taking this patch (and alternatives if risky): very low
String or IDL/UUID changes made by this patch: none

Comment 19

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/457495c9e1cf
https://hg.mozilla.org/releases/mozilla-aurora/rev/97957d719a7e

https://hg.mozilla.org/releases/mozilla-beta/rev/2cb5b5325abe
https://hg.mozilla.org/releases/mozilla-beta/rev/2ac632b74eb6

Comment 20

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:25.0) Gecko/20100101 Firefox/25.0

Verified as fixed on Firefox 24 beta 5 (buildID: 20130822154523) and latest Aurora (buildID: 20130823004003).

Comment 21

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:25.0) Gecko/20100101 Firefox/25.0

Verified as fixed with latest Aurora 25.0a2 (Build ID: 20130909004001).

Comment 22

[Bogdan Maris, Desktop Test Engineering]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:26.0) Gecko/20100101 Firefox/26.0

Verified as fixed on latest Aurora 26.0a2 (buildID: 20131009004001).