Closed Bug 880753 Opened 7 years ago Closed 7 years ago
Reintroduce public ns
Look And Feel::Use Overlay Scrollbars method
In bug 636564, the public nsLookAndFeel::UseOverlayScrollbars was introduced to conveniently check whether or not overlay scrollbars should be used on a system. It was later made protected because this method would ignore the settings in about:config when a user manually turned off overlay scrollbars by setting ui.useOverlayScrollbars (bug 868396). I'd like to reintroduce the public nsLookAndFeel::UseOverlayScrollbars and move the existing one to nsLookAndFeel::UseOverlayScrollbarsInternal to make it more convenient in widget code to make this check. This patch also fixes a minor problem introduced in bug 873012: nsLookAndFeel::AllowOverlayScrollbarsOverlap() used to ignore a manual override via ui.useOverlayScrollbars in about:config (since it was calling nsLookAndFeel::UseOverlayScrollbars instead of GetInt(eIntID_UseOverlayScrollbars)). This didn't result in any visible misbehavior, but we may as well be consistent and correct here.
Added two missing '()'.
Comment on attachment 759825 [details] [diff] [review] Patch I agree with making UseOverlayScrollbars() once again a public method -- with having it be the method to call to fully answer the question of whether not to use overlay scrollbars (since it consults both the system setting and the about:config preference (if it exists)). I agree that this is less confusing. But I think we need to find a different name for what you've called UseOverlayScrollbarsInternal(). We need something to better indicate what that method actually does -- which is to find the system (0S) setting. How about something like SystemWantsOverlayScrollbars()?
Attachment #759825 - Flags: review?(smichaud) → review+
> But I think we need to find a different name for what you've called > UseOverlayScrollbarsInternal(). We need something to better indicate what > that method actually does -- which is to find the system (0S) setting. How > about something like SystemWantsOverlayScrollbars()? Thanks for pointing that out. Changed method name. Carrying over r+.
Target Milestone: mozilla23 → mozilla24
Comment on attachment 759894 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 868396 User impact if declined: No impact on users. Makes it easier for developers to check whether or not a system uses overlay scrollbars. Testing completed (on m-c, etc.): Has been on m-c for a few days. Risk to taking this patch (and alternatives if risky): very low String or IDL/UUID changes made by this patch: none
Attachment #759894 - Flags: approval-mozilla-aurora?
Attachment #759894 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Requesting checkin for aurora.
You need to log in before you can comment on or make changes to this bug.