Bug 90052

Summary:

REGRESSION (r116203): overflow sections don’t have scrollbars

Product:

WebKit

Reporter:

mitz

Component:

Layout and Rendering

Assignee:

Julien Chaffraix <jchaffraix>

Status:

RESOLVED FIXED

Severity:

Major

CC:

adele, bdakin, darin, eric, jchaffraix, simon.fraser, webkit.review.bot

Priority:

Keywords:

InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.7

URL:

data:text/html,<div style="border: solid; height: 100px; overflow: auto;">a<div style="height: 300px;"></div>z</div>

Attachments:

Description	Flags
Proposed blind fix, would appreciate if someone could test it on Lion.	none
Proposed fix 1: Make RenderLayer with overlay scrollbars self-painting.	none
Patch for landing	none

mitz

Reported 2012-06-27 00:16:10 PDT

<rdar://problem/11757381> The overlay scrollbars in overflow: scroll blocks never show, so it’s impossible to tell that they can scroll. To reproduce: navigate to the URL. Notice that as the page loads, the scrollbar doesn’t flash in the top right, and as you scroll the overflow section, the scrollbar doesn’t appear. This was caused by <http://trac.webkit.org/r116203>.

Attachments
Proposed blind fix, would appreciate if someone could test it on Lion. (2.24 KB, patch) 2012-06-28 07:28 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Proposed fix 1: Make RenderLayer with overlay scrollbars self-painting. (4.48 KB, patch) 2012-07-10 18:30 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Patch for landing (4.51 KB, patch) 2012-07-11 09:21 PDT, Julien Chaffraix	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

mitz

Comment 1 2012-06-27 00:17:17 PDT

(In reply to comment #0) > This was caused by <http://trac.webkit.org/r116203>. …which was the fix for bug 85678.

mitz

Comment 2 2012-06-27 00:36:39 PDT

Resize corners (for resize: both) also don’t paint sometimes.

Adele Peterson

Comment 3 2012-06-27 09:46:09 PDT

Should we roll out r116203 and re-assess the optimization?

Simon Fraser (smfr)

Comment 4 2012-06-27 09:49:17 PDT

Yes.

Julien Chaffraix

Comment 5 2012-06-27 12:02:17 PDT

(In reply to comment #3) > Should we roll out r116203 and re-assess the optimization? If it is rolled out, the has self-painting layer descendant optimization should be rolled out as well due to the same issue. The fix should be fairly simple though: the current definition of self-painting layer doesn't seem to cover all the cases (scrollbars, resizing, ...). I will see if I can get something by EOD, if not, it's better to roll it out as I won't be able to look into it before some time.

Julien Chaffraix

Comment 6 2012-06-28 06:53:17 PDT

(In reply to comment #5) > (In reply to comment #3) > > Should we roll out r116203 and re-assess the optimization? > > If it is rolled out, the has self-painting layer descendant optimization should be rolled out as well due to the same issue. > > The fix should be fairly simple though: the current definition of self-painting layer doesn't seem to cover all the cases (scrollbars, resizing, ...). I will see if I can get something by EOD, if not, it's better to roll it out as I won't be able to look into it before some time. Scratch that comment, I spoke too early. I couldn't reproduce the issue on Mac SL or Linux Chomium so it should be related to overlay scrollbars. The bad news is that I have no Lion machine to test but I think I have a blind fix for that that I will post.

Julien Chaffraix

Comment 7 2012-06-28 07:28:09 PDT

Created attachment 149953 [details] Proposed blind fix, would appreciate if someone could test it on Lion.

mitz

Comment 8 2012-06-29 10:38:06 PDT

Comment on attachment 149953 [details] Proposed blind fix, would appreciate if someone could test it on Lion. With the patch applied, I hit ASSERTION FAILED: isSelfPaintingLayer() || hasSelfPaintingLayerDescendant() in RenderLayer.cpp:2995 as soon as I open the test case or cause a <textarea> to have a scroll bar.

Julien Chaffraix

Comment 9 2012-07-10 11:24:39 PDT

(In reply to comment #8) > (From update of attachment 149953 [details]) > With the patch applied, I hit ASSERTION FAILED: isSelfPaintingLayer() || hasSelfPaintingLayerDescendant() in RenderLayer.cpp:2995 as soon as I open the test case or cause a <textarea> to have a scroll bar. That's because I forgot to update this ASSERT. I got my hand on a Lion machine and unfortunately the test case seemed to work fine (both ToT and a nightly build). Is there some special option that I need to enable to see the issue?

mitz

Comment 10 2012-07-10 15:11:27 PDT

(In reply to comment #9) > (In reply to comment #8) > > (From update of attachment 149953 [details] [details]) > > With the patch applied, I hit ASSERTION FAILED: isSelfPaintingLayer() || hasSelfPaintingLayerDescendant() in RenderLayer.cpp:2995 as soon as I open the test case or cause a <textarea> to have a scroll bar. > > That's because I forgot to update this ASSERT. I got my hand on a Lion machine and unfortunately the test case seemed to work fine (both ToT and a nightly build). Is there some special option that I need to enable to see the issue? You need to have overlay scrollbars enabled. In System Preferences > General, select “Show scrollbars: When scrolling”.

Julien Chaffraix

Comment 11 2012-07-10 18:10:46 PDT

Comment on attachment 149953 [details] Proposed blind fix, would appreciate if someone could test it on Lion. > You need to have overlay scrollbars enabled. In System Preferences > General, select “Show scrollbars: When scrolling”. Terrific Dan, that did the trick! I have understood the regression and will upload a fix shortly.

Julien Chaffraix

Comment 12 2012-07-10 18:30:29 PDT

Created attachment 151569 [details] Proposed fix 1: Make RenderLayer with overlay scrollbars self-painting.

Simon Fraser (smfr)

Comment 13 2012-07-10 18:33:32 PDT

Comment on attachment 151569 [details] Proposed fix 1: Make RenderLayer with overlay scrollbars self-painting. Why is this specific to overlay scrollbars?

Julien Chaffraix

Comment 14 2012-07-10 18:43:48 PDT

(In reply to comment #13) > (From update of attachment 151569 [details]) > Why is this specific to overlay scrollbars? Non-overlay scrollbars are painted by their renderer as part of the regular paint() code path (see RenderBlock::paint). Overlay scrollbars are treated differently and painted in a follow-up phase in FrameView::paintContents. The RenderLayer code also has this distinction: we don't call paintOverflowControl in paintLayer if we don't have overlay scrollbars.

Simon Fraser (smfr)

Comment 15 2012-07-10 20:55:35 PDT

Comment on attachment 151569 [details] Proposed fix 1: Make RenderLayer with overlay scrollbars self-painting. View in context: https://bugs.webkit.org/attachment.cgi?id=151569&action=review > Source/WebCore/ChangeLog:11 > + After r120395 (follow-up of r116203), we ignore subtree that have no self-painting layers for subtrees (plural)?

Julien Chaffraix

Comment 16 2012-07-11 09:20:39 PDT

Comment on attachment 151569 [details] Proposed fix 1: Make RenderLayer with overlay scrollbars self-painting. View in context: https://bugs.webkit.org/attachment.cgi?id=151569&action=review >> Source/WebCore/ChangeLog:11 >> + After r120395 (follow-up of r116203), we ignore subtree that have no self-painting layers for > > subtrees (plural)? You are right, we may skip several subtrees.

Julien Chaffraix

Comment 17 2012-07-11 09:21:52 PDT

Created attachment 151716 [details] Patch for landing

WebKit Review Bot

Comment 18 2012-07-11 10:51:10 PDT

Comment on attachment 151716 [details] Patch for landing Clearing flags on attachment: 151716 Committed r122342: <http://trac.webkit.org/changeset/122342>

WebKit Review Bot

Comment 19 2012-07-11 10:51:15 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.