Summary: | Subpixel layout broken with spans with CSS position other than static | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Behdad Esfahbod <behdad> | ||||||||||||||||||||||
Component: | Layout and Rendering | Assignee: | Behdad Esfahbod <behdad> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | dglazkov, eae, eric.carlson, eric, feature-media-reviews, leviw, rbyers, rjkroege, webkit.review.bot | ||||||||||||||||||||||
Priority: | P1 | ||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||
Bug Blocks: | 60318, 90281 | ||||||||||||||||||||||||
Attachments: |
|
Description
Behdad Esfahbod
2012-06-27 13:26:50 PDT
Created attachment 149790 [details]
Patch
Attachment 149790 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:8: Line contains tab character. [whitespace/tab] [5]
Source/WebCore/ChangeLog:10: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 2 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
The change itself looks good, apart form the style violations. Could you add a test for it though please? One based on the minimal test case will do. I'll gladly lend a hand to convert it to a test if you'd like. Thanks Emil. Let me also upload the second patch, then talk about format and tests :). Comment on attachment 149790 [details] Patch Attachment 149790 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13090910 Created attachment 149798 [details]
Patch
Ok. Uploaded second patch. I'll update them tomorrow based on feedback and resubmit. Thanks! Attachment 149798 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:11: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5]
Total errors found: 1 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 149798 [details] Patch Attachment 149798 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13090923 Comment on attachment 149798 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=149798&action=review > Source/WebCore/platform/FractionalLayoutUnit.h:89 > + static FractionalLayoutUnit fromFloatCeil(float value) are there unit tests for FractionalLayoutUnit? If so, it would be nice to add one? (In reply to comment #10) > (From update of attachment 149798 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=149798&action=review > > > Source/WebCore/platform/FractionalLayoutUnit.h:89 > > + static FractionalLayoutUnit fromFloatCeil(float value) > > are there unit tests for FractionalLayoutUnit? If so, it would be nice to add one? No, we don't really have unit tests in webkit. It's all LayoutTests. Comment on attachment 149798 [details] Patch Attachment 149798 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13090950 New failing tests: fast/html/details-marker-style.html svg/zoom/page/zoom-replaced-intrinsic-ratio-001.htm media/video-zoom-controls.html svg/zoom/page/zoom-img-preserveAspectRatio-support-1.html svg/zoom/page/zoom-svg-through-object-with-absolute-size.xhtml svg/zoom/page/zoom-svg-through-object-with-percentage-size.xhtml Created attachment 149821 [details]
Archive of layout-test-results from ec2-cr-linux-04
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 150208 [details]
LayoutTest
I wrote a layout test for this. Figuring out how to integrate it into the patch now.
Created attachment 150228 [details]
Patch
Comment on attachment 150228 [details] Patch Attachment 150228 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13121371 Comment on attachment 150228 [details] Patch Attachment 150228 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13118527 New failing tests: fast/html/details-marker-style.html svg/zoom/page/zoom-replaced-intrinsic-ratio-001.htm media/video-zoom-controls.html svg/zoom/page/zoom-svg-through-object-with-absolute-size.xhtml svg/zoom/page/zoom-svg-through-object-with-percentage-size.xhtml fast/sub-pixel/size-of-span-with-different-positions.html Created attachment 150307 [details]
Archive of layout-test-results from ec2-cr-linux-01
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 150591 [details]
Patch
Created attachment 150592 [details]
Patch
Fixed the win-ews build issue, and the expected-output of the newly added tests. Comment on attachment 150592 [details] Patch Attachment 150592 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13129490 New failing tests: svg/zoom/page/zoom-svg-through-object-with-percentage-size.xhtml svg/zoom/page/zoom-replaced-intrinsic-ratio-001.htm media/video-zoom-controls.html fast/html/details-marker-style.html svg/zoom/page/zoom-svg-through-object-with-absolute-size.xhtml Created attachment 150601 [details]
Archive of layout-test-results from gce-cr-linux-08
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-08 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 150608 [details]
Patch
This looks right to me, but Emil is the best reviewer for this. When he says LGTM, I'm happy to do the official flag marking. :) Comment on attachment 150608 [details]
Patch
LGTM and I like the test, thank you.
Thanks Emil! Eric, can you please press the magic button? Comment on attachment 150608 [details]
Patch
Shazam!
Comment on attachment 150608 [details] Patch Clearing flags on attachment: 150608 Committed r121917: <http://trac.webkit.org/changeset/121917> All reviewed patches have been landed. Closing bug. |