RESOLVED INVALID55737
RenderBox::canBeProgramaticallyScrolled() incorrectly returns 'true' for (i)frames with attribute 'scrolling=no' 
https://bugs.webkit.org/show_bug.cgi?id=55737
Summary RenderBox::canBeProgramaticallyScrolled() incorrectly returns 'true' for (i)f...
Daniel Sievers
Reported 2011-03-03 18:37:00 PST
Also see bug 55257.
Attachments
Patch (2.64 KB, patch)
2011-03-03 19:05 PST, Daniel Sievers 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Daniel Sievers
Comment 1 2011-03-03 18:38:59 PST
The problem is that checking 'node()->isDocumentNode()' is too permissive. The frame might have the attribute scrolling=no set and will never be scrollable. Also see bug 55257, where this would lead to creation of an unwanted compositing layer.
Daniel Sievers
Comment 2 2011-03-03 19:05:25 PST
Created attachment 84681 [details] Patch
James Robinson
Comment 3 2011-03-03 23:25:13 PST
Doing some quick testing it seems that iframes with scrolling=no are programmatically scrollable in Firefox nightlies + the latest Opera stable. My testcase: <!DOCTYPE html> <iframe src="data:text/html;charset=utf-8,<!DOCTYPE html><script>setTimeout('window.scroll(0,10)',500)</script><div style='height:500px'>lala</div>" scrolling=no></iframe> Are you sure we want to change this?
Daniel Sievers
Comment 4 2011-03-04 10:08:11 PST
I see, in that case it's probably ok to allow layers in bug 55257 for that case also. Thanks! (In reply to comment #3) > Doing some quick testing it seems that iframes with scrolling=no are programmatically scrollable in Firefox nightlies + the latest Opera stable. My testcase: > > <!DOCTYPE html> > <iframe src="data:text/html;charset=utf-8,<!DOCTYPE html><script>setTimeout('window.scroll(0,10)',500)</script><div style='height:500px'>lala</div>" scrolling=no></iframe> > > Are you sure we want to change this?
Darin Adler
Comment 5 2011-03-04 16:01:55 PST
Comment on attachment 84681 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84681&action=review > Source/WebCore/ChangeLog:10 > + No new tests. (OOPS!) Why no tests? We normally require tests for all bug fixes.
Darin Adler
Comment 6 2011-03-04 16:02:26 PST
Comment on attachment 84681 [details] Patch I see. This bug was closed. Clearing the review flag, then.
Note You need to log in before you can comment on or make changes to this bug.