Bug 45131

Summary:

REGRESSION (r65539): One pixel white gaps when scrolling Trac changeset pages

Product:

WebKit

Reporter:

Mark Rowe (bdash) <mrowe>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, eric, fsamuel, hyatt, jamesr, mitz, mrowe, rjkroege

Priority:

Keywords:

InRadar, Regression

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

URL:

http://trac.webkit.org/changeset/66679

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	mitz: review-
Patch	darin: review+, webkit.review.bot: commit-queue-
Patch	none

Mark Rowe (bdash)

Reported 2010-09-02 14:09:30 PDT

When scrolling the Trac changeset page at <http://trac.webkit.org/changeset/66679> white one pixel horizontal lines appear in the green portions of the page when I pause and then resume scrolling. * STEPS TO REPRODUCE 1. Load <http://trac.webkit.org/changeset/66679>. 2. Scroll about a third of the way down the page so that the top of your window is half way through the added ChangeLog entry. 3. Scroll down a few lines, then pause for a second, and scroll back up slowly. * RESULT A white gap appears between each table row as you scroll up. * REGRESSION I bisected and tracked this down to r65539. <rdar://problem/8388281>

Attachments
Patch (13.02 KB, patch) 2010-09-10 14:18 PDT, Fady Samuel	no flags	Details Formatted Diff Diff
Patch (12.20 KB, patch) 2010-09-13 14:28 PDT, Fady Samuel	no flags	Details Formatted Diff Diff
Patch (14.40 KB, patch) 2010-09-15 12:22 PDT, Fady Samuel	mitz: review-	Details Formatted Diff Diff
Patch (13.99 KB, patch) 2010-09-29 15:15 PDT, Fady Samuel	darin: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Patch (13.97 KB, patch) 2010-10-05 16:09 PDT, Fady Samuel	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Mark Rowe (bdash)

Comment 1 2010-09-02 14:10:58 PDT

<http://trac.webkit.org/changeset/65539> was the change that introduced this.

Fady Samuel

Comment 2 2010-09-02 14:33:19 PDT

I am seeing the issue. Will look into it. Thanks.

Fady Samuel

Comment 3 2010-09-09 18:37:21 PDT

I have a fix locally. I just have to produce a layout test for it, and then I'll submit a patch. The issue is that the test cases I used to improve dirty cell selection had the border-collapse property set to collapse. In the case where border-collapse is separate, there is an off by one error that occasionally prevents dirty borders from repainting.

Fady Samuel

Comment 4 2010-09-10 14:18:57 PDT

Created attachment 67238 [details] Patch

Fady Samuel

Comment 5 2010-09-13 14:28:42 PDT

Created attachment 67474 [details] Patch

mitz

Comment 6 2010-09-13 14:32:57 PDT

Comment on attachment 67474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=67474&action=prettypatch > LayoutTests/fast/table/simple_paint_separate_borders.html:24 > + divbug.style.visibility = "hidden"; > + if (window.layoutTestController) { > + window.layoutTestController.notifyDone(); > + } > + } > + </script> > +</head> > + <body onload="setTimeout('repaintCell()', 1000)"> > + <div id="bug" style="position: absolute; left: 121px; top: 121px; width: 61px; height: 61px; background-color: red;"></div> I’m surprised that this technique works to test repainting behavior, since DumpRenderTree is supposed to invalidate everything by default. Anyway, even if it works, it appears to take a full second. You should use the standard mechanism for repaint tests instead (see LayoutTests/fast/repaint/resources/repaint.js).

Fady Samuel

Comment 7 2010-09-15 12:22:46 PDT

Created attachment 67699 [details] Patch Updated layout tests according to Mitz' comments.

mitz

Comment 8 2010-09-16 01:11:09 PDT

Comment on attachment 67699 [details] Patch The expected image for the new test doesn’t look right—it looks like the entire view repainted. You also changed an existing test but I don’t see new expected results.

Fady Samuel

Comment 9 2010-09-29 15:15:36 PDT

Created attachment 69258 [details] Patch Fixed expectation pngs and minor tweaking of tests.

WebKit Commit Bot

Comment 10 2010-09-30 00:49:18 PDT

Comment on attachment 69258 [details] Patch Rejecting patch 69258 from commit-queue. Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', '--quiet', 69258]" exit_code: 2 Cleaning working directory Updating working directory Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=69258&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=45131&ctype=xml Processing 1 patch from 1 bug. Processing patch 69258 from bug 45131. Failed to run "[u'/Users/abarth/git/webkit-queue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 9 Full output: http://queues.webkit.org/results/4156031

WebKit Review Bot

Comment 11 2010-10-01 07:59:45 PDT

Comment on attachment 69258 [details] Patch Rejecting patch 69258 from commit-queue. fsamuel@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.

Eric Seidel (no email)

Comment 12 2010-10-01 08:58:59 PDT

It looks like the patch fails to apply. Sorry it's missing some output, we're fixing that.

Fady Samuel

Comment 13 2010-10-01 10:13:29 PDT

(In reply to comment #12) > It looks like the patch fails to apply. Sorry it's missing some output, we're fixing that. Is there something I can do?

Eric Seidel (no email)

Comment 14 2010-10-01 11:31:44 PDT

Yes, you need to post a patch which applies to tip of tree. :)

Fady Samuel

Comment 15 2010-10-05 16:09:58 PDT

Created attachment 69862 [details] Patch

Adam Barth

Comment 16 2010-10-05 16:55:34 PDT

Committed r69161: <http://trac.webkit.org/changeset/69161>

Julien Chaffraix

Comment 17 2011-08-29 17:34:30 PDT

*** Bug 47162 has been marked as a duplicate of this bug. ***

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