Focus event dispatched in iframe causes parent document to scroll incorrectly

[Aaron Chu]

Created attachment 281025 [details]
a test case to demonstrate the bug

On a very long page, when the focus event is dispatched in an iframe to focus an element within the frame, the browser scrolls the page so that the iframe becomes visible when the scrollY begins at the top of the document (0). If the scrollY begins with a value > 0, the browser incorrectly scroll pass the frame.

[Simon Fraser (smfr)]

We do have code that tries to suppress scrolling:

#if PLATFORM(IOS)
    // Focusing a form element triggers animation in UIKit to scroll to the right position.
    // Calling updateFocusAppearance() would generate an unnecessary call to ScrollView::setScrollPosition(),
    // which would jump us around during this animation. See <rdar://problem/6699741>.
    FrameView* view = document().view();
    bool isFormControl = view && is<HTMLFormControlElement>(*this);
    if (isFormControl)
        view->setProhibitsScrolling(true);
#endif
    updateFocusAppearance(restorePreviousSelection ? SelectionRestorationMode::Restore : SelectionRestorationMode::SetDefault);
#if PLATFORM(IOS)
    if (isFormControl)
        view->setProhibitsScrolling(false);
#endif

[Simon Fraser (smfr)]

Ah, I think the problem is rather obvious. view->setProhibitsScrolling() is only called on the inner FrameView, not all the ancestor ones, so the ancestor ones scroll, and with incorrect offsets.

[James Craig]

<rdar://problem/26521616>

[Simon Fraser (smfr)]

I plan to fix this but am having some problems getting a reliable testcase for iOS.

[Simon Fraser (smfr)]

Created attachment 281498 [details]
Patch

[Enrica Casucci]

Comment on attachment 281498 [details]
Patch

Looks good.

[Simon Fraser (smfr)]

https://trac.webkit.org/r202147

[Ryan Haddad]

This change appears to have caused test failures on iso-simulator

Regressions: Unexpected text-only failures (9)
  editing/input/caret-at-the-edge-of-input.html [ Failure ]
  fast/forms/input-text-scroll-left-on-blur.html [ Failure ]
  fast/forms/textarea-no-scroll-on-blur.html [ Failure ]
  fast/forms/textarea-scrolled-type.html [ Failure ]
  fast/forms/textfield-outline.html [ Failure ]
  fast/overflow/scroll-nested-positioned-layer-in-overflow.html [ Failure ]
  fast/overflow/scrollRevealButton.html [ Failure ]
  fast/scrolling/ios/scrollTo-at-page-load.html [ Failure ]
  fast/transforms/scrollIntoView-transformed.html [ Failure ]

<https://build.webkit.org/builders/Apple%20iOS%209%20Simulator%20Release%20WK2%20%28Tests%29/builds/6606>

[Simon Fraser (smfr)]

Thanks, I will investigate

[Simon Fraser (smfr)]

I think these are probably because I disabled both overflow and frame scrolling, but we should really allow overflow scrolling.

[Alexey Proskuryakov]

We can't have 9 tests failing of so long, this should have been rolled out. Will do it now.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 158867

[Alexey Proskuryakov]

What's particularly unfortunate is that some of these tests were already flaky.

[Simon Fraser (smfr)]

Created attachment 281675 [details]
Patch

[Simon Fraser (smfr)]

https://trac.webkit.org/r202243

[Alexey Proskuryakov]

This broke Windows build:

C:\cygwin\home\buildbot\slave\win-release\build\Source\WebKit\win\WebView.cpp(3975): error C2664: 'void WebCore::FrameSelection::revealSelection(WebCore::SelectionRevealMode,const WebCore::ScrollAlignment &,WebCore::RevealExtentOption)': cannot convert argument 1 from 'const WebCore::ScrollAlignment' to 'WebCore::SelectionRevealMode' [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebKit\WebKit.vcxproj]
  C:\cygwin\home\buildbot\slave\win-release\build\Source\WebKit\win\WebView.cpp(3975): note: No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

And iOS tests:

Regressions: Unexpected text-only failures (1)
  fast/overflow/scrollRevealButton.html [ Failure ]

Why was this patch landed without waiting for EWS?

[WebKit Commit Bot]

Re-opened since this is blocked by bug 158972

[Alexey Proskuryakov]

Rolled out in https://trac.webkit.org/r202263.

[Simon Fraser (smfr)]

https://trac.webkit.org/r202292

[Yann Armelin]

This issue still occurs on Safari 11 and last Webkit version, only if the target element is a non-input HTML elements (like div or span).

It can also be reproduced when element.scrollIntoView is called.

[Yann Armelin]

Created attachment 341898 [details]
new test case

[Jon Lee]

(In reply to Yann Armelin from comment #21)
> Created attachment 341898 [details]
> new test case

Because of the age of this bug, it's closed. Yann, would you mind filing a new bug containing your test case? Thank you.

[Yann Armelin]

Sure, new bug : https://bugs.webkit.org/show_bug.cgi?id=186268