Bug 61339 – Can't scroll scaled page that has overflow:hidden on its root

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

Can't scroll scaled page that has overflow:hidden on its root

[Sam Weinig]

<rdar://problem/9029189>

[Sam Weinig]

Created attachment 94548 [details]
Patch

[WebKit Review Bot]

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.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Darin Adler]

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
> [phone]  Sam Weinig  <[e-mail]>
> +
> +        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.
> +
> [phone]  Sam Weinig  <[e-mail]>
> +
> +        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.

[Sam Weinig]

(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.

[Sam Weinig]

 > 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.

[Sam Weinig]

Created attachment 94647 [details]
Patch

[Anders Carlsson]

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

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Sam Weinig]

Committed r87187: <http://trac.webkit.org/changeset/87187>

[Adam Klein]

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.