Frameset scaling is broken
Created attachment 88119 [details] Patch
This patches addresses the following issues: #1 iframes and frameset backgrounds and scroll areas are doubly scaled when they are styled with -webkit-transform. #2 frameset does not respect -webkit-transform-origin when scaled. I will post a new patch shortly with updated layout tests.
Created attachment 88694 [details] Patch
Created attachment 88715 [details] Patch
Comment on attachment 88715 [details] Patch Please add a couple of 3D transform tests. Looking at the code, I'm not convinced the right thing will happen when 3D transforms are present.
I actually see that you are only doing scaling and no other transform 2D or 3D (translate or rotate). You really should test for those cases.
This is all very fragile code. You need to test Mac WebKit1, Mac WebKit2 and other platforms, test hit testing on iframes and plugins etc etc.
I'm working on another iteration of this. Additionally, I have fixed some hit testing bugs and will upload a patch that includes those in another bug. For the time being, I have confirmed that these tests seem to render correctly on Chromium Linux, Chromium Mac AND WebKit Mac's minibrowser! The tests seem to have problems in the WebKit Layout tests on Mac however. What differs between the layout tests and the Minibrowser that might account for this? (In reply to comment #7) > This is all very fragile code. You need to test Mac WebKit1, Mac WebKit2 and other platforms, test hit testing on iframes and plugins etc etc.
Minibrowser is using WebKit2, which is viewless (no native widgets for iframes). DRT is using WebKit1, which has native views for iframes.
(In reply to comment #9) > Minibrowser is using WebKit2, which is viewless (no native widgets for iframes). DRT is using WebKit1, which has native views for iframes. So I have figured out how to position the RenderFrame correctly, and in fact it seems to render correctly, but a good chunk of it appears to be clipped off. This appears to be a WebKit1 Mac specific issue. Where does clipping happen in WebKit1 Mac? Thanks.
(In reply to comment #10) > (In reply to comment #9) > > Minibrowser is using WebKit2, which is viewless (no native widgets for iframes). DRT is using WebKit1, which has native views for iframes. > > > So I have figured out how to position the RenderFrame correctly, and in fact it seems to render correctly, but a good chunk of it appears to be clipped off. This appears to be a WebKit1 Mac specific issue. Where does clipping happen in WebKit1 Mac? Thanks. It seems the clipping on WebKit1 Mac is determined by WebFrameView::visibleRect in Source/WebKit/mac/WebView/WebFrameView.mm which does not correctly take into account transforms.
(In reply to comment #9) > Minibrowser is using WebKit2, which is viewless (no native widgets for iframes). DRT is using WebKit1, which has native views for iframes. This patch (copied and pasted changelog) sees to address a similar problem on WebKit1 Mac but it doesn't address framesets. Continuing to investigate.
(In reply to comment #12) > (In reply to comment #9) > > Minibrowser is using WebKit2, which is viewless (no native widgets for iframes). DRT is using WebKit1, which has native views for iframes. > > This patch (copied and pasted changelog) sees to address a similar problem on WebKit1 Mac but it doesn't address framesets. Continuing to investigate. Sorry, I somehow pressed commit instead of copying and pasting: commit 448c931f6d9fc828515a8ba489335af221a2622b Author: simon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Date: Mon Jan 17 01:07:24 2011 +0000 2011-01-16 Simon Fraser <simon.fraser@apple.com> Reviewed by Dan Bernstein. Issues with iframes and plugins when the WebView is scaled. <rdar://problem/6213380> When _scaleWebView has been called on a WebView, iframes in WebKit1 render and hit-test incorrectly, and plug-ins don't scale up. This is caused by AppKit NSViews not playing nicely with the scale applied through style. Work around most of these issues by adjusting the bounds size of widgets to allow iframe contents to paint with the correct scale, and fix various places in the code where we relied on coordinate transforms via NSViews (which ignore CSS transforms). * WebCore.exp.in: * platform/Widget.cpp: (WebCore::Widget::setBoundsSize): * platform/Widget.h: * platform/mac/WidgetMac.mm: (WebCore::Widget::setBoundsSize): (WebCore::Widget::paint): * rendering/RenderLayerCompositor.cpp: (WebCore::RenderLayerCompositor::shouldPropagateCompositingToEnclosingIFrame): * rendering/RenderWidget.cpp: (WebCore::RenderWidget::setWidgetGeometry): (WebCore::RenderWidget::setWidget): (WebCore::RenderWidget::updateWidgetPosition): * rendering/RenderWidget.h: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75897 268f45cc-cd09-0410-ab3c-d52691b4dbfc
The underlying issue here is that the Widget hierarchy is ignorant of transforms. Anywhere we get a widget's position or bounds is doing to do the wrong thing when there are CSS transforms above it. The right approach would be to eliminate geometry from Widget entirely.
<rdar://problem/9673843>
Created attachment 103434 [details] Patch
(In reply to comment #9) > Minibrowser is using WebKit2, which is viewless (no native widgets for iframes). DRT is using WebKit1, which has native views for iframes. (In reply to comment #16) > Created an attachment (id=103434) [details] > Patch Simon, this has been tested on WebKit2 Mac, and Chromium Linux platforms (included layout tests). It does not fix the problem on WebKit1 Mac. Is this current state of the patch acceptable? I need to fix the changelog, but before I take time to do that, I'll let you take a look at this patch. Thanks. Hit testing issues have been fixed in RenderFrameSet.cpp, bool RenderFrameSet::userResize(MouseEvent* evt). For the purposes of this bug, I will remove those changes but I decided to post everything together so you know a fix is on its way, I don't expect you to give me an r+ for this. Rather, I'd just like your OK for all but the hit testing part, and I'll move the hit testing part to another bug report tomorrow, and add hit testing layout tests. Thanks,
Comment on attachment 103434 [details] Patch Attachment 103434 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9337479 New failing tests: fast/overflow/scrollRevealButton.html
I think the patch is going in the right direction.
Created attachment 103493 [details] Patch
Comment on attachment 103493 [details] Patch Attachment 103493 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9332831 New failing tests: fast/overflow/scrollRevealButton.html
(In reply to comment #21) > (From update of attachment 103493 [details]) > Attachment 103493 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/9332831 > > New failing tests: > fast/overflow/scrollRevealButton.html Sigh..I did not break this test, it was broken from before..
(In reply to comment #22) > (In reply to comment #21) > > (From update of attachment 103493 [details] [details]) > > Attachment 103493 [details] [details] did not pass chromium-ews (chromium-xvfb): > > Output: http://queues.webkit.org/results/9332831 > > > > New failing tests: > > fast/overflow/scrollRevealButton.html > > Sigh..I did not break this test, it was broken from before.. *bump*
Comment on attachment 103493 [details] Patch Rejecting attachment 103493 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: : Unexpected image and text mismatch : (2) fast/overflow/scrollRevealButton.html = IMAGE+TEXT svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT Regressions: Unexpected image mismatch : (5) fast/text/atsui-multiple-renderers.html = IMAGE fast/text/international/danda-space.html = IMAGE fast/text/international/thai-baht-space.html = IMAGE fast/text/international/thai-line-breaks.html = IMAGE platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE Full output: http://queues.webkit.org/results/9351320
Comment on attachment 103493 [details] Patch Rejecting attachment 103493 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: : Unexpected image and text mismatch : (2) fast/overflow/scrollRevealButton.html = IMAGE+TEXT svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT Regressions: Unexpected image mismatch : (5) fast/text/atsui-multiple-renderers.html = IMAGE fast/text/international/danda-space.html = IMAGE fast/text/international/thai-baht-space.html = IMAGE fast/text/international/thai-line-breaks.html = IMAGE platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE Full output: http://queues.webkit.org/results/9371203
Comment on attachment 103493 [details] Patch Rejecting attachment 103493 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2 Last 500 characters of output: : Unexpected image and text mismatch : (2) fast/overflow/scrollRevealButton.html = IMAGE+TEXT svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT Regressions: Unexpected image mismatch : (5) fast/text/atsui-multiple-renderers.html = IMAGE fast/text/international/danda-space.html = IMAGE fast/text/international/thai-baht-space.html = IMAGE fast/text/international/thai-line-breaks.html = IMAGE platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE Full output: http://queues.webkit.org/results/9352636
(In reply to comment #26) > (From update of attachment 103493 [details]) > Rejecting attachment 103493 [details] from commit-queue. > > Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2 > > Last 500 characters of output: > : Unexpected image and text mismatch : (2) > fast/overflow/scrollRevealButton.html = IMAGE+TEXT > svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT > > Regressions: Unexpected image mismatch : (5) > fast/text/atsui-multiple-renderers.html = IMAGE > fast/text/international/danda-space.html = IMAGE > fast/text/international/thai-baht-space.html = IMAGE > fast/text/international/thai-line-breaks.html = IMAGE > platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE > > > > Full output: http://queues.webkit.org/results/9352636 It looks like the only broken test that may potentially be my fault is fast/overflow/scrollRevealButton.html...investigating this now. If it's not my fault, I'll manually land this patch.
*** Bug 64992 has been marked as a duplicate of this bug. ***
Comment on attachment 103493 [details] Patch Trying to submit to the commit queue on more time. I still believe this doesn't break anything new but we'll see.
Comment on attachment 103493 [details] Patch Rejecting attachment 103493 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: svg-fonts-word-spacing.html = IMAGE+TEXT Regressions: Unexpected image mismatch : (5) fast/text/atsui-multiple-renderers.html = IMAGE fast/text/international/danda-space.html = IMAGE fast/text/international/thai-baht-space.html = IMAGE fast/text/international/thai-line-breaks.html = IMAGE platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE Regressions: Unexpected text diff mismatch : (1) http/tests/inspector/resource-har-conversion.html = TEXT Full output: http://queues.webkit.org/results/9404300
(In reply to comment #30) > (From update of attachment 103493 [details]) > Rejecting attachment 103493 [details] from commit-queue. > > Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 > > Last 500 characters of output: > svg-fonts-word-spacing.html = IMAGE+TEXT > > Regressions: Unexpected image mismatch : (5) > fast/text/atsui-multiple-renderers.html = IMAGE > fast/text/international/danda-space.html = IMAGE > fast/text/international/thai-baht-space.html = IMAGE > fast/text/international/thai-line-breaks.html = IMAGE > platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE > > Regressions: Unexpected text diff mismatch : (1) > http/tests/inspector/resource-har-conversion.html = TEXT > > > > Full output: http://queues.webkit.org/results/9404300 I've manually confirmed that all these tests are broken in HEAD on chromium-linux. Will commit this manually.
Comment on attachment 103493 [details] Patch Rejecting attachment 103493 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: : Unexpected image and text mismatch : (2) fast/overflow/scrollRevealButton.html = IMAGE+TEXT svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT Regressions: Unexpected image mismatch : (5) fast/text/atsui-multiple-renderers.html = IMAGE fast/text/international/danda-space.html = IMAGE fast/text/international/thai-baht-space.html = IMAGE fast/text/international/thai-line-breaks.html = IMAGE platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE Full output: http://queues.webkit.org/results/9402437
Comment on attachment 103493 [details] Patch Rejecting attachment 103493 [details] from commit-queue. Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2 Last 500 characters of output: : Unexpected image and text mismatch : (2) fast/overflow/scrollRevealButton.html = IMAGE+TEXT svg/custom/svg-fonts-word-spacing.html = IMAGE+TEXT Regressions: Unexpected image mismatch : (5) fast/text/atsui-multiple-renderers.html = IMAGE fast/text/international/danda-space.html = IMAGE fast/text/international/thai-baht-space.html = IMAGE fast/text/international/thai-line-breaks.html = IMAGE platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE Full output: http://queues.webkit.org/results/9403949
> I've manually confirmed that all these tests are broken in HEAD on chromium-linux. Will commit this manually. Dollars to donuts this patch will break a test.
(In reply to comment #34) > > I've manually confirmed that all these tests are broken in HEAD on chromium-linux. Will commit this manually. > > Dollars to donuts this patch will break a test. Sigh, these tests seem to break on chromium-linux regardless...I don't really know how to proceed...
> Sigh, these tests seem to break on chromium-linux regardless...I don't really know how to proceed... It seems likely that your patch breaks fast/overflow/scrollRevealButton.html. Do you have a Linux system? If so, you can try to reproduce the failure locally.
(In reply to comment #36) > > Sigh, these tests seem to break on chromium-linux regardless...I don't really know how to proceed... > > It seems likely that your patch breaks fast/overflow/scrollRevealButton.html. Do you have a Linux system? If so, you can try to reproduce the failure locally. I believe I tried that exact test locally without my change yesterday and it was still failing...I'll retest again today to confirm.
> I believe I tried that exact test locally without my change yesterday and it was still failing...I'll retest again today to confirm. Keep in mind that tests can fail in different ways. You're welcome, of course, to land you patch, but I suspect the main tree will tell you the same thing.
(In reply to comment #38) > > I believe I tried that exact test locally without my change yesterday and it was still failing...I'll retest again today to confirm. > > Keep in mind that tests can fail in different ways. > > You're welcome, of course, to land you patch, but I suspect the main tree will tell you the same thing. Curiouser and curiouser, the test seems to be passing with and without my change now...cleaning out my output directory and doing a full build for sanity now.
Created attachment 104226 [details] Patch
Comment on attachment 104226 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=104226&action=review > LayoutTests/fast/frames/frame-set-scaling-centered.html:4 > + <frame src="data:text/html,<body bgcolor='green'></body>"> > + <frame src="data:text/html,<body bgcolor='green'></body>"> All these tests would look cleaner with border:none style on the frames
(In reply to comment #41) > (From update of attachment 104226 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=104226&action=review > > > LayoutTests/fast/frames/frame-set-scaling-centered.html:4 > > + <frame src="data:text/html,<body bgcolor='green'></body>"> > > + <frame src="data:text/html,<body bgcolor='green'></body>"> > > All these tests would look cleaner with border:none style on the frames If layout tests are still failing (review bot will tell us soon) then I'll fix them in this patch. If all tests are passing, I still have a hit testing fix coming soon so I'll add a slew of new tests and I'll clean-up existing scaling tests at that point.
Comment on attachment 104226 [details] Patch Clearing flags on attachment: 104226 Committed r93287: <http://trac.webkit.org/changeset/93287>
All reviewed patches have been landed. Closing bug.