Closed
Bug 791676
Opened 12 years ago
Closed 12 years ago
[New Tab Page] don't start transition for dynamically created elements off a timeout
Categories
(Firefox :: Tabbed Browser, defect)
Firefox
Tabbed Browser
Tracking
()
RESOLVED
FIXED
Firefox 18
People
(Reporter: ttaubert, Assigned: ttaubert)
Details
Attachments
(1 file, 1 obsolete file)
958 bytes,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
The new tab page updater uses setTimeout(0) to start transitions for dynamically created elements: https://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/updater.js#174 After doing some testing and looking at bug 701626 it seems that this method doesn't work in 95% of all cases - which I somehow didn't notice when implementing it like this.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #661777 -
Flags: review?(jaws)
Comment 2•12 years ago
|
||
Comment on attachment 661777 [details] [diff] [review] use getComputedStyle() before setting the transitioned property to compute the element's current style >- // Without the setTimeout() the node would just appear instead of fade in. >- setTimeout(function () { >- gTransformation.showSite(site, function () batch.pop()); >- }, 0); >+ // Compute the element's current style before showing it again. >+ window.getComputedStyle(site.node).getPropertyValue("opacity"); >+ gTransformation.showSite(site, function () batch.pop()); This comment doesn't really tell what's going on... It should say that style changes are being flushed in order to make the fade-in transition work.
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #2) > This comment doesn't really tell what's going on... It should say that style > changes are being flushed in order to make the fade-in transition work. Yes, that comment was poorly chosen, sorry.
Attachment #661777 -
Attachment is obsolete: true
Attachment #661777 -
Flags: review?(jaws)
Attachment #661799 -
Flags: review?(jaws)
Comment 4•12 years ago
|
||
Comment on attachment 661799 [details] [diff] [review] use getComputedStyle() to flush all style changes to make the transition work >+ // Use getComputedStyle() to flush all style changes for the >+ // dynamically inserted site to make the fade-in transition work. "Use getComputedStyle() to" is redundant, just looking at the code you're commenting on makes this already clear. >+ window.getComputedStyle(site.node).getPropertyValue("opacity"); window.getComputedStyle(site.node).opacity;
Attachment #661799 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Thank you! https://hg.mozilla.org/integration/fx-team/rev/fff9c3a1028b
Whiteboard: [fixed-in-fx-team]
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fff9c3a1028b
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 18
You need to log in
before you can comment on or make changes to this bug.
Description
•