| Summary: | Add seamless layout code (and pass all the remaining seamless tests) | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||||||||||||||||||||
| Component: | New Bugs | Assignee: | Eric Seidel (no email) <eric> | ||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | abarth, cmarcelo, dbates, dglazkov, hyatt, inferno, jamesr, jchaffraix, koivisto, macpherson, menard, ojan, tony, webkit.review.bot, zalan | ||||||||||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||||||
| Bug Depends on: | 87363 | ||||||||||||||||||||||||||||||
| Bug Blocks: | 45950 | ||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||
|
Description
Eric Seidel (no email)
2012-05-16 04:37:37 PDT
Created attachment 142225 [details]
Patch
This is the real interesting part of seamless. I'm very interested in hearing from other layout experts on if they agree with my implementation approach here. I should note that I also filed a w3c bug when preparing this patch: https://www.w3.org/Bugs/Public/show_bug.cgi?id=17076 about how best to handle scrollbars, etc. Comment on attachment 142225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142225&action=review > LayoutTests/fast/frames/seamless/resources/two-inline-blocks.html:6 > +/* FIXME: This vertical-align should not be needed, but without it the contentHeight() > +of the document is reported as 4px too large, likely due to implied line decent? */ > +div { vertical-align: top; } This is kinda an odd bug. Without this the FrameView::contentHeight() reports 4px larger than I expected. It's possible there is a better way to work around this. the <html> element itself does not grow to include this presumed descent. I guess contentHeight() includes this implicit line "overflow" for descent? Comment on attachment 142225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=142225&action=review > Source/WebCore/rendering/RenderIFrame.cpp:186 > + // So we set our height to min(max-height, UINT_MAX) to avoid generating unecessary > + // vertical scrollbars then we'll shrink the height after child layout. > + LayoutUnit maxHeight = MAX_LAYOUT_UNIT; > + if (!style()->logicalMaxHeight().isUndefined()) > + maxHeight = std::min<LayoutUnit>(maxHeight, style()->logicalMaxHeight().value()); > + setLogicalHeight(maxHeight); If folks have better suggestions as to how to get this behavior, I'd very much like to hear them. It's possible I could use the supressScrollbars() functionality, but that seems tied to firstLayout and I seem skeptical that it's the right approach. > Source/WebCore/rendering/RenderIFrame.cpp:190 > + FrameView* childFrameView = static_cast<FrameView*>(widget()); > + childFrameView->layout(); This is likely not needed. The layout actually happens inside updateWidgetPosition(). :) But I'm matching the RenderFrameBase::layoutFrameFlattening path here. Note that I don't have any of the computePrefWidths code here like frame-flattening does, since those calls are made as part of computeLogicalWidth() as per normal layout. > Source/WebCore/rendering/RenderIFrame.cpp:198 > + computeLogicalHeight(); It may be cleaner to split out a separate layoutAsSeamless() function, similar to what layoutWithFlattening has done. We also might consider just splitting seamless frames into a wholly separate render object. Comment on attachment 142225 [details] Patch Attachment 142225 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12715272 Comment on attachment 142225 [details] Patch Attachment 142225 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12709410 Comment on attachment 142225 [details]
Patch
Uploading a new patch to fix release builds.
Created attachment 142230 [details]
Fix release builds and simplify RenderIFrame::layout
Comment on attachment 142230 [details] Fix release builds and simplify RenderIFrame::layout Attachment 142230 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12717257 New failing tests: http/tests/security/javascriptURL/javascriptURL-in-new-iframe.html http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-to-javscript-url.html http/tests/security/seamless/seamless-cross-origin.html http/tests/security/seamless/seamless-sandbox-srcdoc.html http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html fast/frames/iframe-option-crash.xhtml http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html http/tests/appcache/credential-url.html fast/frames/javascript-url-for-deleted-frame.html fast/frames/adopt-from-created-document.html http/tests/appcache/different-scheme.html http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-sub-frame-2-level.html plugins/iframe-shims.html Created attachment 142287 [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
Comment on attachment 142230 [details] Fix release builds and simplify RenderIFrame::layout View in context: https://bugs.webkit.org/attachment.cgi?id=142230&action=review > Source/WebCore/rendering/RenderIFrame.cpp:59 > + LayoutUnit border = borderTop() + borderBottom(); borderTop and borderBottom both return ints so no need to change this. > Source/WebCore/rendering/RenderIFrame.cpp:128 > + // FIXME: Is this always a valid cast? What about plugins? How about adding an assert instead of the FIXME? ASSERT(widget()->isFrameView()); > Source/WebCore/rendering/RenderIFrame.cpp:168 > + // So we set our height to min(max-height, UINT_MAX) to avoid generating unecessary s/UNIT_MAX/MAX_LAYOUT_UNIT/ > Source/WebCore/rendering/RenderIFrame.cpp:188 > + ASSERT(!childRoot->firstChild() || !childRoot->firstChild()->firstChild() || !childRoot->firstChild()->firstChild()->needsLayout()); This line probably needs an explanation. Created attachment 142378 [details]
Updated per emil's suggestions
I'm still diagnosing the XSS null pointer crashes. I'm not sure what I'm tripping over yet. Comment on attachment 142378 [details]
Updated per emil's suggestions
Due to the XSS null pointer crash, this won't be ready for commit yet, but certainly could have another round of review from layout experts!
Comment on attachment 142378 [details] Updated per emil's suggestions Attachment 142378 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12716418 New failing tests: http/tests/security/javascriptURL/javascriptURL-in-new-iframe.html fast/frames/iframe-option-crash.xhtml fast/frames/javascript-url-for-deleted-frame.html http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-from-javscript-url.html http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-to-javscript-url.html http/tests/security/javascriptURL/xss-ALLOWED-from-javascript-url-sub-frame-2-level.html http/tests/appcache/credential-url.html fast/frames/adopt-from-created-document.html plugins/iframe-shims.html http/tests/security/javascriptURL/xss-ALLOWED-to-javascript-url-sub-frame-2-level.html http/tests/appcache/different-scheme.html Created attachment 142388 [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 142442 [details]
Information about the crash
I think the crash comes because setDocument() does not expect the attach() to end up recusing through and resolving style on the parent document, which finds that it doesn't know if it can access the child document at this exact moment, as it doesn't yet have a security origin? I wonder if we could trigger this crash another way. What seems to be happening is that we're recalculating style on the parent document in between the time that the child document has been created and when it gets its security origin. Unclear if Frame::setDocument really wants to be attaching the document at this time. Stopping that would stop the crash. I could also remove the "resolve style on our parent before resolving on ourself" logic for now. It may not be necessary. I also could add a null check to either SecurityOrigin::canAccess or JSDOMWindowBase::allowsAccessFromPrivate, both of which could be reasonable. Created attachment 142445 [details]
now with less crash
Comment on attachment 142445 [details] now with less crash View in context: https://bugs.webkit.org/attachment.cgi?id=142445&action=review > Source/WebCore/rendering/RenderIFrame.cpp:197 > + RenderPart::computeLogicalWidth(); > + RenderPart::computeLogicalHeight(); These don't need to be explicit calls to RenderPart::, but I left them the way they were. > Source/WebCore/rendering/RenderIFrame.cpp:-120 > - RenderPart::layout(); RenderPart::layout() doesn't actually do anything. :) So I removed this useless call. Comment on attachment 142445 [details] now with less crash View in context: https://bugs.webkit.org/attachment.cgi?id=142445&action=review I briefly looked over this patch and noticed some issues. I didn't verify the correctness of the layout logic. > Source/WebCore/ChangeLog:14 > + - Seamless support shrink-wrap negotation when the iframe is inline. Nit: negotation => negotiation (Is it a negotiation or an algorithm?) Nit: This sentence doesn't read well. Maybe we need to make some words plural? like support? > Source/WebCore/dom/Document.cpp:1726 > + // hits a null-derefence due to security code always assuming the document has a SecurityOrigin. Nit: null-derefence => null-dereference > Source/WebCore/rendering/RenderIFrame.cpp:129 > + ASSERT(widget()->isFrameView()); We need to check that widget() is non-null here since the rest of this function assumes widget() can return a null pointer. > Source/WebCore/rendering/RenderIFrame.cpp:169 > + // So we set our height to min(max-height, MAX_LAYOUT_UNIT) to avoid generating unecessary Nit: unecessary => unnecessary > Source/WebCore/rendering/RenderIFrame.cpp:179 > + FrameView* childFrameView = static_cast<FrameView*>(widget()); Are we guaranteed that childFrameView will be non-null? Notice that you check that childFrameView is non-null on line 186. > Source/WebCore/rendering/RenderIFrame.cpp:186 > + RenderView* childRoot = childFrameView ? static_cast<RenderView*>(childFrameView->frame()->contentRenderer()) : 0; See remark on line 179. > Source/WebCore/rendering/RenderIFrame.cpp:201 > return; > + } else if (isSeamless()) { Nit: This should be a "if" statement instead of an "else if" because of the early return above. (In reply to comment #23) > (From update of attachment 142445 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=142445&action=review > > Source/WebCore/rendering/RenderIFrame.cpp:201 > > return; > > + } else if (isSeamless()) { > > Nit: This should be a "if" statement instead of an "else if" because of the early return above. I've left this as-is, as the early-return is wrong. I just don't have tests to prove its wrong. :) Created attachment 142553 [details]
now with more splelnig fixes
Comment on attachment 142553 [details] now with more splelnig fixes View in context: https://bugs.webkit.org/attachment.cgi?id=142553&action=review I'm almost comfortable r+'ing this. Mainly, I'd like to see the vertical writing mode tests for the scary bit in layoutSeamlessly. You really should try to get Hyatt to take a quick look at this patch if you can. If nothing else, just have him verify that the code in layoutSeamlessly is sane, that's the only part that I'm wary of since it's doing something quite different from normal layout methods. > Source/WebCore/page/FrameView.cpp:2928 > + if (iframeRenderer->flattenFrame() || iframeRenderer->isSeamless()) Doesn't this change behavior for frame flattening if frameFlatteningEnabled is false? i.e. we will frame flatten even though the setting was false. I think you need to check !m_frame->settings() || !m_frame->settings()->frameFlatteningEnabled() here as well. > Source/WebCore/rendering/RenderIFrame.cpp:174 > + // Normally we would set our height to 0 before laying out our kids. > + // However RenderView (the root of the child document's rendering tree) > + // assumes its container is of a fixed size, not dynamically grown to it. > + // So we set our height to min(max-height, MAX_LAYOUT_UNIT) to avoid generating unnecessary > + // vertical scrollbars then we'll shrink the height after child layout. > + LayoutUnit maxHeight = MAX_LAYOUT_UNIT; > + if (!style()->logicalMaxHeight().isUndefined()) > + maxHeight = std::min<LayoutUnit>(maxHeight, style()->logicalMaxHeight().value()); > + setLogicalHeight(maxHeight); This scares me, e.g., I highly doubt it does the right thing with vertical writing mode. You could add some vertical writing mode tests to verify. Specifically, write tests with enough content to wrap into multiple lines *and* overflow horizontally out of their container. Make sure they render the same as a vertical writing mode test that uses a div. In fact, you could write these as reftests! :) > Source/WebCore/rendering/RenderIFrame.cpp:178 > + // Replaced elements do not respect padding, so we just add border to the child's height. I'm on the fence as to whether seamless iframes should respect padding. I think they probably should. It's not a big deal either way. Can you just add a FIXME for now? > Source/WebCore/rendering/RenderIFrame.cpp:204 > + // Note we do not return so as to share the layer and overflow updates below. Pet peeve nit: s/Note we/We/ :) > Source/WebCore/rendering/RenderIFrame.cpp:207 > + // No kids to layout as a replaced element. Grammar nit: No kids to layout as *this is* a replaced element. Example vertical writing mode case to test: http://plexode.com/eval3/#ht=%3Cdiv%20style%3D%22width%3A%2050px%3B%20height%3A%20100px%3B%20border%3A%203px%20solid%22%3E%0A%3Cdiv%20style%3D%22-webkit-writing-mode%3Avertical-lr%3B%20background-color%3A%20red%22%3ELorem%20ipsum%20dolor%20and%20some%20other%20latin...%3C%2Fdiv%3E%0A%3C%2Fdiv%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1 Comment on attachment 142553 [details] now with more splelnig fixes View in context: https://bugs.webkit.org/attachment.cgi?id=142553&action=review >> Source/WebCore/page/FrameView.cpp:2928 >> + if (iframeRenderer->flattenFrame() || iframeRenderer->isSeamless()) > > Doesn't this change behavior for frame flattening if frameFlatteningEnabled is false? i.e. we will frame flatten even though the setting was false. I think you need to check !m_frame->settings() || !m_frame->settings()->frameFlatteningEnabled() here as well. RenderIframe::flattenFrame() already checks the setting. But I'm happy to add a comment to that effect? If we were flattening iframes when we shouldn't a zillion tests would fail. :) >> Source/WebCore/rendering/RenderIFrame.cpp:174 >> + setLogicalHeight(maxHeight); > > This scares me, e.g., I highly doubt it does the right thing with vertical writing mode. You could add some vertical writing mode tests to verify. Specifically, write tests with enough content to wrap into multiple lines *and* overflow horizontally out of their container. Make sure they render the same as a vertical writing mode test that uses a div. In fact, you could write these as reftests! :) I'm not sure what you mean. The fact that I'm setting logical height instead of height? Or do you mean how I'm tryign to avoid scrollbars but only doing so in teh height direction? >> Source/WebCore/rendering/RenderIFrame.cpp:178 >> + // Replaced elements do not respect padding, so we just add border to the child's height. > > I'm on the fence as to whether seamless iframes should respect padding. I think they probably should. It's not a big deal either way. Can you just add a FIXME for now? Sure. Done. Comment on attachment 142553 [details] now with more splelnig fixes View in context: https://bugs.webkit.org/attachment.cgi?id=142553&action=review >>> Source/WebCore/page/FrameView.cpp:2928 >>> + if (iframeRenderer->flattenFrame() || iframeRenderer->isSeamless()) >> >> Doesn't this change behavior for frame flattening if frameFlatteningEnabled is false? i.e. we will frame flatten even though the setting was false. I think you need to check !m_frame->settings() || !m_frame->settings()->frameFlatteningEnabled() here as well. > > RenderIframe::flattenFrame() already checks the setting. But I'm happy to add a comment to that effect? If we were flattening iframes when we shouldn't a zillion tests would fail. :) oic. That's fine either way then. Are there really many tests for frame flattening? :) >>> Source/WebCore/rendering/RenderIFrame.cpp:174 >>> + setLogicalHeight(maxHeight); >> >> This scares me, e.g., I highly doubt it does the right thing with vertical writing mode. You could add some vertical writing mode tests to verify. Specifically, write tests with enough content to wrap into multiple lines *and* overflow horizontally out of their container. Make sure they render the same as a vertical writing mode test that uses a div. In fact, you could write these as reftests! :) > > I'm not sure what you mean. The fact that I'm setting logical height instead of height? Or do you mean how I'm tryign to avoid scrollbars but only doing so in teh height direction? Setting logicalHeight is definitely correct. The fact that you're setting height to non-zero is what concerns me. Here's a case I'm worried we would do the wrong thing with text wrapping: http://plexode.com/eval3/#ht=%3Cdiv%20style%3D%22-webkit-writing-mode%3A%20vertical-lr%3B%20outline%3A%202px%20solid%20blue%3B%20width%3A%20100px%22%3E%0A%20%20%20%20%3Cdiv%20style%3D%22-webkit-writing-mode%3A%20horizontal-tb%3B%20background-color%3A%20red%22%3E%0A%20%20%20%20%20%20%20%20foo%20bar%20baz%20foo%0A%20%20%20%20%3C%2Fdiv%3E%0A%3C%2Fdiv%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1 I'm worried that with the above code we'll not wrap the text at all, where we should be wrapping at 100px. A test case would help appease my worries, but it really would be good to get Hyatt's signoff as well. I don't think we need to block committing this patch on Hyatt review if you add a test-case like the one above, but at least put in a FIXME that this might do the wrong thing until we can have Hyatt verify it's sanity. (In reply to comment #29) > (From update of attachment 142553 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=142553&action=review > Setting logicalHeight is definitely correct. The fact that you're setting height to non-zero is what concerns me. Here's a case I'm worried we would do the wrong thing with text wrapping: > > http://plexode.com/eval3/#ht=%3Cdiv%20style%3D%22-webkit-writing-mode%3A%20vertical-lr%3B%20outline%3A%202px%20solid%20blue%3B%20width%3A%20100px%22%3E%0A%20%20%20%20%3Cdiv%20style%3D%22-webkit-writing-mode%3A%20horizontal-tb%3B%20background-color%3A%20red%22%3E%0A%20%20%20%20%20%20%20%20foo%20bar%20baz%20foo%0A%20%20%20%20%3C%2Fdiv%3E%0A%3C%2Fdiv%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1 > > I'm worried that with the above code we'll not wrap the text at all, where we should be wrapping at 100px. A test case would help appease my worries, but it really would be good to get Hyatt's signoff as well. I don't think we need to block committing this patch on Hyatt review if you add a test-case like the one above, but at least put in a FIXME that this might do the wrong thing until we can have Hyatt verify it's sanity. That test case displays differently on TOT vs. shipping Safari. The outer div has 0 height on TOT/Chrome, where as it has 100% height on shiping Safari. (In reply to comment #30) > (In reply to comment #29) > > (From update of attachment 142553 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=142553&action=review > > Setting logicalHeight is definitely correct. The fact that you're setting height to non-zero is what concerns me. Here's a case I'm worried we would do the wrong thing with text wrapping: > > > > http://plexode.com/eval3/#ht=%3Cdiv%20style%3D%22-webkit-writing-mode%3A%20vertical-lr%3B%20outline%3A%202px%20solid%20blue%3B%20width%3A%20100px%22%3E%0A%20%20%20%20%3Cdiv%20style%3D%22-webkit-writing-mode%3A%20horizontal-tb%3B%20background-color%3A%20red%22%3E%0A%20%20%20%20%20%20%20%20foo%20bar%20baz%20foo%0A%20%20%20%20%3C%2Fdiv%3E%0A%3C%2Fdiv%3E&ohh=1&ohj=1&jt=&ojh=1&ojj=1&ms=100&oth=0&otj=0&cex=1 > > > > I'm worried that with the above code we'll not wrap the text at all, where we should be wrapping at 100px. A test case would help appease my worries, but it really would be good to get Hyatt's signoff as well. I don't think we need to block committing this patch on Hyatt review if you add a test-case like the one above, but at least put in a FIXME that this might do the wrong thing until we can have Hyatt verify it's sanity. > > That test case displays differently on TOT vs. shipping Safari. The outer div has 0 height on TOT/Chrome, where as it has 100% height on shiping Safari. Yup, the behavior change was intentional from http://trac.webkit.org/changeset/97654. Created attachment 143128 [details]
Fixed per Ojan's review
After discussion with Ojan over IRC, we'll look at possible writing mode bugs after landing the initial implementation. Comment on attachment 143128 [details] Fixed per Ojan's review View in context: https://bugs.webkit.org/attachment.cgi?id=143128&action=review > Source/WebCore/rendering/RenderIFrame.cpp:214 > m_overflow.clear(); > addVisualEffectOverflow(); Side comment: I'm not sure what overflow iframes can ever have. This code is suspicious to me. Not that it should affect seamless in any way. Tony might also have thoughts on what might break by setting logicalHeight to max instead of 0. Comment on attachment 143128 [details] Fixed per Ojan's review Rejecting attachment 143128 [details] from commit-queue. New failing tests: fast/frames/seamless/seamless-inline.html fast/frames/seamless/seamless-float.html Full output: http://queues.webkit.org/results/12744466 Created attachment 143194 [details]
Archive of layout-test-results from ec2-cq-02
The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: ec2-cq-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 143128 [details] Fixed per Ojan's review Attachment 143128 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12742538 New failing tests: fast/frames/seamless/seamless-inline.html fast/frames/seamless/seamless-float.html Created attachment 143225 [details]
Archive of layout-test-results from ec2-cr-linux-02
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
(In reply to comment #35) > Tony might also have thoughts on what might break by setting logicalHeight to max instead of 0. I read the comment in the code, but I don't understand the problem that vertical scrollbars cause. Setting the height to max smells funny to me. There might be another way to accomplish the same goal. Have you asked Hyatt? (In reply to comment #40) > (In reply to comment #35) > > Tony might also have thoughts on what might break by setting logicalHeight to max instead of 0. > > I read the comment in the code, but I don't understand the problem that vertical scrollbars cause. Setting the height to max smells funny to me. There might be another way to accomplish the same goal. Have you asked Hyatt? Hyatt has been CC'd on every seamless change. Ojan has asked him specifically about this function over IRC. I've not seen any response. Certainly his (and anyone elses!) feedback about other ways to avoid scrollbars while still setting the height to 0 at the start are most welcome! I considered using the m_firstLayout flag, but that didn't seem quite right. I could manually disable vertical scrollbars during dthe first layout, but that also didn't seem right. So this was the solution I settled on for now. :) I don't understand. Those tests do not fail for me locally on Mac or CrMac builds. :( (In reply to comment #42) > I don't understand. Those tests do not fail for me locally on Mac or CrMac builds. :( You might want to try with mock scrollbars turned on. That might help roll the dice w.r.t. scroll backs. Created attachment 143675 [details]
Patch for landing
Created attachment 143676 [details]
Patch for landing
Comment on attachment 143676 [details] Patch for landing Clearing flags on attachment: 143676 Committed r118291: <http://trac.webkit.org/changeset/118291> All reviewed patches have been landed. Closing bug. |