Use frame scheduling for animated Fullscreen toolbar hiding

RESOLVED FIXED in Firefox 12

Status

()

Firefox
General
--
enhancement
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: Thomas Stache, Assigned: dao)

Tracking

({perf, polish})

Trunk
Firefox 12
perf, polish
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [snappy])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3) Gecko/20100805 Firefox/4.0
Build Identifier: 

Follow-up according to https://bugzilla.mozilla.org/show_bug.cgi?id=240859#c39

> The animation should be replaced with the new css transition, reducing the
> custom animation code for this fullscreen autohide function.

Reproducible: Always
(Reporter)

Updated

7 years ago
Depends on: 240859

Updated

7 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 1

7 years ago
Morphing. mozRequestAnimationFrame provides the same benefits as CSS animations but leaves more control to the script.
Summary: Use CSS Transition for Fullscreen toolbar show/hide → Use frame scheduling for animated Fullscreen toolbar hiding
(Assignee)

Updated

7 years ago
Severity: trivial → enhancement
Keywords: perf, polish
(Assignee)

Updated

7 years ago
Component: Toolbars → General
QA Contact: toolbars → general
(Assignee)

Comment 2

7 years ago
Created attachment 468001 [details] [diff] [review]
patch
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #468001 - Flags: review?(mano)
(Assignee)

Comment 3

7 years ago
Created attachment 468022 [details] [diff] [review]
patch

more s/FullScreen/this/
Attachment #468001 - Attachment is obsolete: true
Attachment #468022 - Flags: review?(mano)
Attachment #468001 - Flags: review?(mano)

Comment 4

6 years ago
Dao, is there any perf win to be had by moving to CSS or is this just for code maintainability?

Comment 5

6 years ago
This is currently the jerkiest of the UI animations for me and I believe that mozRequestAnimationFrame should help with that. Also, this bug has the perf keyword.

I think after 10+ months it's probably time to look for another reviewer.
(Assignee)

Updated

6 years ago
Whiteboard: [snappy]
(Assignee)

Comment 6

6 years ago
Created attachment 585017 [details] [diff] [review]
patch

updated to tip
Attachment #468022 - Attachment is obsolete: true
Attachment #585017 - Flags: review?(dolske)
Attachment #468022 - Flags: review?(mano)
(Assignee)

Comment 7

6 years ago
Created attachment 585018 [details] [diff] [review]
patch

oops, I didn't pay enough attention when merging
Attachment #585017 - Attachment is obsolete: true
Attachment #585018 - Flags: review?(dolske)
Attachment #585017 - Flags: review?(dolske)
(Assignee)

Updated

6 years ago
Attachment #585018 - Flags: review?(dietrich)
Comment on attachment 585018 [details] [diff] [review]
patch

Review of attachment 585018 [details] [diff] [review]:
-----------------------------------------------------------------

looks good, r=me. i <3 bind().
Attachment #585018 - Flags: review?(dietrich) → review+
(Assignee)

Updated

6 years ago
Attachment #585018 - Flags: review?(dolske)
(Assignee)

Comment 9

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/2843998a1579
Target Milestone: --- → Firefox 12
https://hg.mozilla.org/mozilla-central/rev/2843998a1579
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Created attachment 587407 [details] [diff] [review]
Followup patch

The animation is also cancelled in enterDomFullScreen, which was missed.

If preferred, we can re-close this & just file a followup.
Attachment #587407 - Flags: review?(dao)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 12

6 years ago
(In reply to Paul O'Shannessy [:zpao] from comment #11)
> If preferred, we can re-close this & just file a followup.

Please do.
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Attachment #587407 - Attachment is obsolete: true
Attachment #587407 - Flags: review?(dao)
Depends on: 717030
You need to log in before you can comment on or make changes to this bug.