Bug 55737

Summary:

RenderBox::canBeProgramaticallyScrolled() incorrectly returns 'true' for (i)frames with attribute 'scrolling=no'

Product:

WebKit

Reporter:

Daniel Sievers <sievers>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

jamesr

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none

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	Details Formatted Diff Diff
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.