Bug 45131 - REGRESSION (r65539): One pixel white gaps when scrolling Trac changeset pages
Summary: REGRESSION (r65539): One pixel white gaps when scrolling Trac changeset pages
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P1 Normal
Assignee: Nobody
URL: http://trac.webkit.org/changeset/66679
Keywords: InRadar, Regression
: 47162 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-02 14:09 PDT by Mark Rowe (bdash)
Modified: 2011-08-29 17:34 PDT (History)
9 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 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>
Comment 1 Mark Rowe (bdash) 2010-09-02 14:10:58 PDT
<http://trac.webkit.org/changeset/65539> was the change that introduced this.
Comment 2 Fady Samuel 2010-09-02 14:33:19 PDT
I am seeing the issue. Will look into it. Thanks.
Comment 3 Fady Samuel 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.
Comment 4 Fady Samuel 2010-09-10 14:18:57 PDT
Created attachment 67238 [details]
Patch
Comment 5 Fady Samuel 2010-09-13 14:28:42 PDT
Created attachment 67474 [details]
Patch
Comment 6 mitz 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).
Comment 7 Fady Samuel 2010-09-15 12:22:46 PDT
Created attachment 67699 [details]
Patch

Updated layout tests according to Mitz' comments.
Comment 8 mitz 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.
Comment 9 Fady Samuel 2010-09-29 15:15:36 PDT
Created attachment 69258 [details]
Patch

Fixed expectation pngs and minor tweaking of tests.
Comment 10 WebKit Commit Bot 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
Comment 11 WebKit Review Bot 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.
Comment 12 Eric Seidel (no email) 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.
Comment 13 Fady Samuel 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?
Comment 14 Eric Seidel (no email) 2010-10-01 11:31:44 PDT
Yes, you need to post a patch which applies to tip of tree. :)
Comment 15 Fady Samuel 2010-10-05 16:09:58 PDT
Created attachment 69862 [details]
Patch
Comment 16 Adam Barth 2010-10-05 16:55:34 PDT
Committed r69161: <http://trac.webkit.org/changeset/69161>
Comment 17 Julien Chaffraix 2011-08-29 17:34:30 PDT
*** Bug 47162 has been marked as a duplicate of this bug. ***