[Webkit-unassigned] [Bug 228302] macOS key-driven smooth scrolling does not stop when focus changes

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=228302

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #440879|review+                     |review-
              Flags|                            |

--- Comment #24 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 440879
  --> https://bugs.webkit.org/attachment.cgi?id=440879
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=440879&action=review

> Source/WebCore/page/Page.cpp:2384
> +        for (auto& scrollableArea : *scrollableAreas)
> +            scrollableArea->stopScrollAnimations();

This will stop smooth scroll animations and possibly rubberband animations too, which I guess is OK (they are short).

You'll also need to call stopAsyncAnimatedScroll() to prepare for the keyboard scroller running on the scrolling thread (the code is not well factored; ScrollableArea should have a common function for stopping sync and async scrolls).

> Source/WebCore/platform/ScrollingEffectsController.cpp:97
> +void ScrollingEffectsController::stopScrollAnimations()
> +{
> +    if (m_isAnimatingKeyboardScrolling)
> +        m_client.keyboardScrollingAnimator()->handleKeyUpEvent();
> +}

I think it's wrong for ScrollingEffectsController::stopScrollAnimations() to only stop keyboard animations. This class now has smooth scroll and will have rubberband animations.

Also, ScrollingEffectsController::stopAnimatedScroll() and ScrollingEffectsController::stopKeyboardScrolling() already exist.

Either change the whole call chain from Page down to be about keyboard scrolling, or keep the names and call the existing stopAnimatedScroll().

> Tools/DumpRenderTree/mac/EventSendingController.mm:1088
> +    RetainPtr<ModifierKeys> modifierKeys = [ModifierKeys modifierKeysWithKey:character modifiers:buildModifierFlags(modifiers) keyLocation:location];
> +
> +    [[[mainFrame frameView] documentView] layout];
> +
> +#if !PLATFORM(IOS_FAMILY)
> +    auto event = retainPtr([NSEvent keyEventWithType:NSEventTypeKeyUp
> +        location:NSMakePoint(5, 5)
> +        modifierFlags:modifierKeys->modifierFlags
> +        timestamp:[self currentEventTime]
> +        windowNumber:[[[mainFrame webView] window] windowNumber]
> +        context:[NSGraphicsContext currentContext]
> +        characters:modifierKeys->eventCharacter.get()
> +        charactersIgnoringModifiers:modifierKeys->charactersIgnoringModifiers.get()
> +        isARepeat:NO
> +        keyCode:modifierKeys->keyCode]);
> +#else
> +    auto event = adoptNS([[WebEvent alloc] initWithKeyEventType:WebEventKeyUp timeStamp:[self currentEventTime] characters:modifierKeys->eventCharacter.get() charactersIgnoringModifiers:modifierKeys->charactersIgnoringModifiers.get() modifiers:(WebEventFlags)modifierKeys->modifierFlags isRepeating:NO withFlags:0 withInputManagerHint:nil keyCode:[character characterAtIndex:0] isTabKey:([character characterAtIndex:0] == '\t')]);
> +#endif

Lots of sharable code here.

> Tools/WebKitTestRunner/InjectedBundle/Bindings/EventSendingController.idl:49
> +    undefined rawKeyDown(DOMString key, object modifierArray, long location);

I'd like to see a comment explaining what "raw" means.

> Tools/WebKitTestRunner/mac/EventSenderProxy.mm:695
> +    RetainPtr<ModifierKeys> modifierKeys = [ModifierKeys modifierKeysWithKey:toWTFString(key) modifiers:buildModifierFlags(modifiers) keyLocation:keyLocation];
> +
> +    NSEvent *event = [NSEvent keyEventWithType:NSEventTypeKeyUp
> +        location:NSMakePoint(5, 5)
> +        modifierFlags:modifierKeys->modifierFlags
> +        timestamp:absoluteTimeForEventTime(currentEventTime())
> +        windowNumber:[m_testController->mainWebView()->platformWindow() windowNumber]
> +        context:[NSGraphicsContext currentContext]
> +        characters:modifierKeys->eventCharacter.get()
> +        charactersIgnoringModifiers:modifierKeys->charactersIgnoringModifiers.get()
> +        isARepeat:NO
> +        keyCode:modifierKeys->keyCode];

Ditto for code sharing.

> LayoutTests/fast/scrolling/unfocusing-page-while-keyboard-scrolling.html:1
> +<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true EventHandlerDrivenSmoothKeyboardScrollingEnabled=true ] -->

Has useFlexibleViewport but you're only enabling the test on macOS? useFlexibleViewport only applies to iOS.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211013/07e1f0ce/attachment.htm>