Summary: | Can't scroll scaled page that has overflow:hidden on its root | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||||||
Component: | New Bugs | Assignee: | Sam Weinig <sam> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | adamk, bdakin, dglazkov, webkit.review.bot | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Sam Weinig
2011-05-23 19:25:27 PDT
Created attachment 94548 [details]
Patch
Attachment 94548 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1
Source/WebCore/page/FrameView.cpp:523: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
Source/WebCore/page/FrameView.cpp:542: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4]
Total errors found: 2 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 94548 [details] Patch Attachment 94548 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8658037 New failing tests: fast/block/float/overhanging-tall-block.html fast/backgrounds/size/contain-and-cover.html http/tests/misc/acid2.html fast/repaint/iframe-scroll-repaint.html fast/overflow/overflow_hidden.html fast/block/positioning/vertical-rl/fixed-positioning.html fast/css/acid2.html fast/block/positioning/rtl-fixed-positioning.html svg/custom/image-rescale-scroll.html fast/events/scroll-in-scaled-page-with-overflow-hidden.html Created attachment 94568 [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: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Comment on attachment 94548 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94548&action=review I’m going to say r=me but this probably needs more refinement. >> Source/WebCore/page/FrameView.cpp:523 >> + case OHIDDEN: { > > A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] No need to add braces. > Source/WebCore/page/FrameView.cpp:527 > + if (m_frame->pageScaleFactor() != 1) > + hMode = ScrollbarAuto; > + else > + hMode = ScrollbarAlwaysOff; I think you want to check if the scale factor is > 1. I don’t think that != is quite right. Also, I think this logic is right for the top level frame, but not sure it’s helpful for nested frames. > LayoutTests/ChangeLog:19 > +2011-05-23 Sam Weinig <sam@webkit.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Need a short description and bug URL (OOPS!) > + > + * fast/events/scroll-in-scaled-page-with-overflow-hidden-expected.txt: Added. > + * fast/events/scroll-in-scaled-page-with-overflow-hidden.html: Added. > + > +2011-05-23 Sam Weinig <sam@webkit.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Can't scroll scaled page that has overflow:hidden on its root > + https://bugs.webkit.org/show_bug.cgi?id=61339 > + > + * fast/events/scroll-in-scaled-page-with-overflow-hidden-expected.txt: Added. > + * fast/events/scroll-in-scaled-page-with-overflow-hidden.html: Added. > + Double change log is not good. (In reply to comment #6) > (From update of attachment 94548 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=94548&action=review > > I’m going to say r=me but this probably needs more refinement. There are a couple tests failing, so I definitely need another round. I will address your feedback as well. Thanks for the review. > There are a couple tests failing, so I definitely need another round. I will address your feedback as well. Thanks for the review. The test failures were from setting hMode for the vertical case. Silly me. Created attachment 94647 [details]
Patch
Comment on attachment 94647 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=94647&action=review > Source/WebCore/page/FrameView.cpp:519 > + // To combat the inablity to scroll on a page with overflow:hidden on the root when scaled, disregard hidden when inablity -> inability Comment on attachment 94647 [details] Patch Attachment 94647 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8730536 New failing tests: fast/events/scroll-in-scaled-page-with-overflow-hidden.html Created attachment 94652 [details]
Archive of layout-test-results from ec2-cr-linux-03
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Committed r87187: <http://trac.webkit.org/changeset/87187> Note: this test fails on Chromium WebKit due to lack of eventSender.scalePageBy (already filed as https://bugs.webkit.org/show_bug.cgi?id=58013). I'll add a line to test_expectations.txt for it. |