316992 - onscroll event performance drastically reduced from FF 1.0.x to 1.5

Status: NEW

(Core :: DOM: Events, defect, P5)

Description

[Michael Krax]

User-Agent:       Mozilla/4.0 (compatible; MSIE 7.0b; Windows NT 5.1; .NET CLR 1.1.4322; .NET CLR 2.0.50215; Avalon 6.0.4030; WinFX RunTime 1.0.50215)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051111 Firefox/1.5

The performance of the onscroll event has drasticly reduced from FF 1.0.x to 1.5 - resulting in a lot of "in the wild" scripts behave odd.





Reproducible: Always

Steps to Reproduce:
The script below shows an example of a typical implementation of a "header" for a scrollable list which should scroll horizontal but not vertical (something like position-y fixed).

In FF 1.0.x the div does not move while scrolling, it's virtually fixed in vertical position. In FF 1.5 the div moves and jumps while scrolling fast.



<div onscroll="document.getElementById('positionFixedY').style.top=this.scrollTop" style="position:absolute; border: 1px solid #000000; overflow:auto; left:10px; top:10px; width:500px; height:500px;">

    <div id="positionFixedY" style="position:absolute; width:2000px; background-color:red;">
    1 ----------------------------  
    2 ----------------------------   
    3 ----------------------------  
    4 ----------------------------  
    5 ---------------------------- 
    </div>

    x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>
 x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>
    x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>x<br>

</div>

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

This regressed between 2005-08-21 and 2005-08-22
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-08-21+14&maxdate=2005-08-22+08&cvsroot=%2Fcvsroot
So I would think this is a regression from bug 189308, somehow.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It's probably just because we moved the event to be fired asynchronously instead of synchronously. I'm not sure what we can do about this; there are good reasons to fire it asynchronously.

Comment 4

[Michael Krax]

(In reply to comment #3)
If the current trend for web applications keeps on going more and more people will create complex websites that use custom event handling for various stuff. e.g. positioning, overlays, drag and drop, etc.

Reliable and pixel exact events are key for stuff like that. Internet Explorer has also a speed problem with this example, but you can use "javascript in styles" to get the events without a delay.

This is really a major regression, not for ordinary websites but for browser based applications. Why should the style "position:fixed" be faster than a javascript doing the same thing? Of course, from a browser developers point of view it might be a great difference how events get handled (styles are probably much closer to the rendering engine than a script lanuguage on top of everyting) - but from an application developers point of view it just seems javascript is somehow broken and slow. 

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The big difference is that Javascript can do arbitrary things, like destroy the element that's currently being scrolled or even then entire window. That is prone to cause crashes (possibly even exploitable ones) unless we're very careful. Making the event asynchronous so it fires after we're done scrolling means that script execution is delayed until it's safe to run. "position:fixed" can't do anything other than what it says.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

There probably are ways to make this better though, without going back to synchronous event dispatch. I'll think about it.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Attachment: safer synchronous dispatching

Just a quick hack. With this, scroll event is dispatched much more often. https://bugzilla.mozilla.org/attachment.cgi?id=140108 can be used to test that.

Because nsIScrollPositionListeners aren't always refcounted, 
adding a wrapper which can be kept alive should help with some
of the possible crashers.

The testcase in this bug doesn't still quite work, at least not in
Linux (I wonder whether it has ever worked properly).
But the red box stays closer to the top of the scrollable area.

(nsIScrollPositionListener.h seems to use tabs and spaces, the .diff 
 looks a bit bad.)

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Attachment: previous patch was missing one file

Comment 9

[hlomas]

Please address this. For web application development there is a huge difference between the scroll event firing every redraw versus scrollstart and scrollend.

Comment 10

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, 4 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.