Summary: | REGRESSION (r52839): Incomplete repaint of IMG with text-align:center | ||
---|---|---|---|
Product: | WebKit | Reporter: | Cory Mintz <cory> |
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | abarth, commit-queue, dbates, eae, eric, jamesr, leviw, mitz, ojan, pierre.lafayette, simon.fraser, tony, webkit.review.bot |
Priority: | P1 | Keywords: | HasReduction, Regression |
Version: | 528+ (Nightly build) | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
Description
Cory Mintz
2010-06-03 14:53:41 PDT
Created attachment 57878 [details]
Reduction
Reduction attached.
The reduction shows an IMG inside of a DIV. The IMG has text-align:center. With the correct sequence of steps the region the IMG would be in if it wasn't centered is repainted, rather than where it actually this. This causes the IMG to be clipped in half.
Note: It's possible that the test case will work on first load and you will need to refresh the page.
Created attachment 57881 [details]
Reduction (Revised)
Made variable names more clear.
Looks (from quartz debug) that the repaint invalidation is the right size/shape but at the wrong offset. If text-align:right is used the image is completely hidden when you refresh, this indicates that the text-align property is not affecting the offset of the dirty rect at all. Also, it is not limited to text-align. This came up because we were using this guys method to vertically center an image: http://www.brunildo.org/test/img_center.html This will cause the image to get cut vertically as well so you just see the top-left quadrant. Created attachment 65941 [details]
Test cased reduced even further; showing example of text-align right hiding image completely
On Chrome 7.0.503.0, loading this file always hides the image completely. Selecting the area around the region makes the image reappear as seen in the demo video.
Created attachment 66290 [details]
This should fix it.
This should fix the issue. James. R's fix didn't really deal with the inline replaced element case. This just generalizes to all inline content.
Will write up a new test and upload the patch soon. Thanks for your help! Created attachment 66412 [details]
Contains two new layout tests as well as the original patch by dhyatt
Adding new layout tests to dhyatt's patch.
Created attachment 66494 [details]
Revised patch with added layout tests
Contains layout tests and the bug fix.
Created attachment 66496 [details]
PNG for block-layout-inline-children-replaced.html test case
Created attachment 66497 [details]
PNG for block-layout-inline-children-float-positioned.html test case
Attachment 66496 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Total errors found: 0 in 0 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 66497 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Total errors found: 0 in 0 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 66548 [details]
Patch
Attachment 66548 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
LayoutTests/ChangeLog:11: Line contains tab character. [whitespace/tab] [5]
LayoutTests/ChangeLog:14: Line contains tab character. [whitespace/tab] [5]
Total errors found: 2 in 9 files
If any of these errors are false positives, please file a bug against check-webkit-style.
LayoutTests/ChangeLog has some indentation issues. WebCore/ChangeLog needs to include a one-line description and link to a bug before the full description. Otherwise, this looks great. Created attachment 66554 [details]
Patch
Comment on attachment 66554 [details]
Patch
The ChangeLog entry needs to include a link to this bug report. The patch is otherwise good.
Created attachment 66557 [details]
Patch
Comment on attachment 66557 [details] Patch Rejecting patch 66557 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 255 Last 500 characters of output: ositioned.html patching file LayoutTests/fast/repaint/block-layout-inline-children-replaced.html patching file LayoutTests/platform/mac/fast/repaint/block-layout-inline-children-float-positioned-expected.checksum literal is only avaliable with the XS version at /Library/Perl/5.8.8/Compress/Zlib.pm line 9 BEGIN failed--compilation aborted at /Library/Perl/5.8.8/Compress/Zlib.pm line 9. Compilation failed in require at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/VCSUtils.pm line 1670. Full output: http://queues.webkit.org/results/3917084 There is a problem with the perl install on the cq machine, which we've not been able to figure out yet. It has trouble with git binary patches as a result. I'll see if I can troubleshoot it again. Comment on attachment 66557 [details] Patch Sorry about the noise. The CQ bot was sick: bug 45723. Comment on attachment 66557 [details] Patch Rejecting patch 66557 from commit-queue. Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 255 Last 500 characters of output: ositioned.html patching file LayoutTests/fast/repaint/block-layout-inline-children-replaced.html patching file LayoutTests/platform/mac/fast/repaint/block-layout-inline-children-float-positioned-expected.checksum literal is only avaliable with the XS version at /Library/Perl/5.8.8/Compress/Zlib.pm line 9 BEGIN failed--compilation aborted at /Library/Perl/5.8.8/Compress/Zlib.pm line 9. Compilation failed in require at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/VCSUtils.pm line 1670. Full output: http://queues.webkit.org/results/4004006 Comment on attachment 66557 [details]
Patch
I believe it's fixed for real this time.
Comment on attachment 66557 [details] Patch Clearing flags on attachment: 66557 Committed r67463: <http://trac.webkit.org/changeset/67463> All reviewed patches have been landed. Closing bug. http://trac.webkit.org/changeset/67463 might have broken GTK Linux 32-bit Debug The following changes are on the blame list: http://trac.webkit.org/changeset/67464 http://trac.webkit.org/changeset/67463 fast/repaint/block-layout-inline-children-float-positioned.html no longer renders quite like it did when you landed it. (Note how the text overlaps now: http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/repaint/block-layout-inline-children-float-positioned-expected.png It looks like the results changed in: http://trac.webkit.org/changeset/96005 Which doesn't make a whole lot of sense to me. Are we sure our rendering is still correct? (In reply to comment #29) > fast/repaint/block-layout-inline-children-float-positioned.html no longer renders quite like it did when you landed it. (Note how the text overlaps now: > http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/repaint/block-layout-inline-children-float-positioned-expected.png > > It looks like the results changed in: > http://trac.webkit.org/changeset/96005 > > Which doesn't make a whole lot of sense to me. > > Are we sure our rendering is still correct? The rendering is clearly wrong. When you resize the window the text jumps from between teh left and the middle. Someone was recently working on align: center iirc. Interesting. Our current rendering matches FF, however FF does not jump the text between two positions when you resize the window like we do. |