Closed Bug 778810 Opened 13 years ago Closed 13 years ago

Show/Hide scrollbars depending on activity

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: vingtetun, Assigned: vingtetun)

References

Details

Attachments

(2 files, 2 obsolete files)

For B2G it would be useful to be able to hide the overlaying scrollbars if they are unactive.
Attached patch Add a ScrollbarActivity class (obsolete) — Splinter Review
This patch is shamelessly stolen from bug 636564 but I have removed all the unneeded parts. It adds a new ScrollbarActivity class that toggle an active flag for scrollbars.
Attachment #647233 - Flags: review?(roc)
Comment on attachment 647233 [details] [diff] [review] Add a ScrollbarActivity class Review of attachment 647233 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/ScrollbarActivity.h @@ +16,5 @@ > + > +namespace mozilla { > +namespace layout { > + > +class ScrollbarActivity : public nsISupports { This needs a big comment explaining what it's for and how it works. Also, it can just live in the mozilla namespace. @@ +33,5 @@ > + void ActivityEnded(); > + void Destroy(); > + > +protected: > + PRInt32 mNestedActivityCounter; This needs to be documented. Also, put the member variables together in the file. @@ +38,5 @@ > + bool IsActivityOngoing() { > + return mNestedActivityCounter > 0; > + } > + > + bool mIsActive; Same here. @@ +41,5 @@ > + > + bool mIsActive; > + void SetIsActive(bool aNewActive); > + > + static const PRUint32 kScrollbarActivityEndedDelay = 450; // milliseconds This won't compile on some compilers. Use an enum instead. ::: layout/generic/nsGfxScrollFrame.h @@ +117,5 @@ > private: > nsGfxScrollFrameInner *mInner; > }; > > + void FinishReflowForScrollbar(nsIContent* aContent, nscoord aMinX, Why did this change to aMinX?
Comment on attachment 647234 [details] [diff] [review] b2g/ part. Show/Hide the scrollbar based on the |active| attribute Review of attachment 647234 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/content.css @@ +46,3 @@ > xul|scrollbar[disabled] { > + opacity: 0; > + -moz-transition: opacity 1s ease; Nit: maybe 1s is a bit long?
Attachment #647234 - Flags: review?(fabrice) → review+
Attached patch Patch v0.2 (obsolete) — Splinter Review
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3) > ::: layout/generic/nsGfxScrollFrame.h > @@ +117,5 @@ > > private: > > nsGfxScrollFrameInner *mInner; > > }; > > > > + void FinishReflowForScrollbar(nsIContent* aContent, nscoord aMinX, > > Why did this change to aMinX? That's a typo, sorry.
Attachment #647233 - Attachment is obsolete: true
Attachment #647233 - Flags: review?(roc)
Attachment #648198 - Flags: review?(roc)
Comment on attachment 648198 [details] [diff] [review] Patch v0.2 Review of attachment 648198 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/ScrollbarActivity.cpp @@ +13,5 @@ > +NS_IMPL_ISUPPORTS0(ScrollbarActivity) > + > +void > +ScrollbarActivity::Destroy() { > + CancelActivityFinishedTimer(); { on new line
Attachment #648198 - Flags: review?(roc) → review+
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #6) > Comment on attachment 648198 [details] [diff] [review] > Patch v0.2 > > Review of attachment 648198 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/generic/ScrollbarActivity.cpp > @@ +13,5 @@ > > +NS_IMPL_ISUPPORTS0(ScrollbarActivity) > > + > > +void > > +ScrollbarActivity::Destroy() { > > + CancelActivityFinishedTimer(); > > { on new line Thanks a lot for the quick review.
Status: NEW → ASSIGNED
Comment on attachment 648198 [details] [diff] [review] Patch v0.2 Review of attachment 648198 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/generic/ScrollbarActivity.h @@ +36,5 @@ > + * ActivityOccured() has been made. When the timeout expires ActivityFinished() > + * is call and reset the active state. > + */ > + > +class ScrollbarActivity : public nsISupports { Actually, why does this have to be refcounted? I think you can and should just make this explicitly managed and give nsGfxScrollFrameInner an nsAutoPtr<ScrollbarActivity>. Instead of Destroy() you can just use the destructor.
Attachment #648198 - Flags: review+ → review-
Attached patch Patch v0.3Splinter Review
Assignee: nobody → 21
Attachment #648198 - Attachment is obsolete: true
Attachment #648206 - Flags: review?(roc)
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
With XFCE with Ubuntu I'm seeing a regression in Firefox nightly where bugzilla, gmail and other pages shows a horizontal scrollbar even when the content fit on window. After use mozregression I get this pushlog http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=20fc34efd733&tochange=9453cf029b72 It is possible that this bug or another one related is causing this regression?
Depends on: 793848
Depends on: 842166
Blocks: 1450883
No longer blocks: 1450883
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: