Created attachment 259067 [details]
[Animated GIF] Bug
See the attached GIF.
I changed the highlight color to black to exaggerate the effect.
For span#time, the animation duration is 1 second, as expected.
All the other elements blink black and white close to 60 times per second.
span#time is different from the rest as it doesn't have any immediate text nodes changing. #minutes, #seconds, and #ms change 60 times per second, so their attributes should always stay highlighted.
Created attachment 259068 [details]
I think that this is more of an issue with the fact that we recreate the DOM tree element anytime there is any sort of change, even if that change happens to not change anything (for example, changing the title of minute from 1 to 1). I'll take a look and see if I can add anything to either prevent this from happening or limit the number of DOM update animations possible per given period of time.
Created attachment 259070 [details]
[Animated GIF] Expected behavior
Same in Chrome DevTools.
Created attachment 259072 [details]
[Animated GIF] Bug 2
(In reply to comment #3)
> I think that this is more of an issue with the fact that we recreate the DOM
> tree element anytime there is any sort of change, even if that change
> happens to not change anything (for example, changing the title of minute
> from 1 to 1). I'll take a look and see if I can add anything to either
> prevent this from happening or limit the number of DOM update animations
> possible per given period of time.
There are two issues: one that you mentioned and one shown on this attached GIF. Even though we don't change textContent, span#ms still blinks and not stays highlighted.
Maybe we should avoid sending DOM change notifications from the backend if the text does not change value. I guess it is getting a new text node, so that might not work. At least then front end should not treat it as a change. Though, that is what the page is doing…
Devin is right, the issue is the tree element is recreated for the text child node change. Then the title attribute changes. When the text child node changes again, the existing attribute change is lost when the parent tree element is recreated again. Rinse and repeat. If only the inner text node is changing, we should be able to update the tree element instead of recreating the parent tree element.
This wouldn't be an issue for long text nodes, since we show those as separate tree elements. I suspect Chrome is doing smart small text node updating when it is combined with the parent node.
Created attachment 350384 [details]
When run from the console, this works fine, but it doesn't have an effect when run inside DOMTreeElement.js 🤔
Created attachment 358113 [details]
Comment on attachment 358113 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=358113&action=review
r=me, this seems to address the majority of flashing issues.
I do still see poor animations on frequently updating elements, perhaps based on the resolution of performance.now()?
Visible in just the seconds element of:
<span id="time" title="?">
<span id="minutes">?</span>:<span id="seconds">?</span>
var date = new Date;
minutes.textContent = minutes.title = date.getMinutes();
seconds.textContent = seconds.title = date.getSeconds();
90% of the time the animation works as expected, but sometimes it does not.
> + element: null,
We could put `listener: null` as well.
Created attachment 363434 [details]
Comment on attachment 363434 [details]
Clearing flags on attachment: 363434
Committed r242322: <https://trac.webkit.org/changeset/242322>
All reviewed patches have been landed. Closing bug.