<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - macOS key-driven smooth scrolling does not stop when focus changes"
   href="https://bugs.webkit.org/show_bug.cgi?id=228302#c3">Comment # 3</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - macOS key-driven smooth scrolling does not stop when focus changes"
   href="https://bugs.webkit.org/show_bug.cgi?id=228302">bug 228302</a>
              from <span class="vcard"><a class="email" href="mailto:thorton@apple.com" title="Tim Horton <thorton@apple.com>"> <span class="fn">Tim Horton</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=435011&action=diff" name="attach_435011" title="Patch">attachment 435011</a> <a href="attachment.cgi?id=435011&action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=435011&action=review">https://bugs.webkit.org/attachment.cgi?id=435011&action=review</a>

<span class="quote">> Source/WebCore/ChangeLog:8
> +        No tests yet.</span >

This seems like a grand time to rectify this, this one should be eminently testable (long page, send keydown, change focus, make sure the scroll stops... eventually (it won't be immediately, this will be a tricky part))

<span class="quote">> Source/WebCore/page/Page.cpp:2356
> +        reportUnfocusedToScrollableAreas();</span >

Something is weird about the name, but I can't quite place it. I think it's because we usually say didX or xChanged (didLoseFocus?). I'm not sure the "to scrollable areas" part belongs in the Page method name; you could imagine using it for other things on focus loss.

(though I do see some precedent for this style too, in e.g. `notifyPageThatContentAreaWillPaint`).

Maybe someone else has opinions :)

<span class="quote">> Source/WebCore/platform/ScrollController.cpp:234
> +        m_client.keyboardScrollingAnimator()->handleKeyUpEvent();</span >

Probably would be ideal if this called a function something like `stopScrollingWithAnimation`, and make `handleKeyUpEvent`'s implementation also call that. Not great to call something named `keyUp` not in response to a `keyUp` event, that's just confusing.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>