Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should require Input event to be fired within same event loop as keydown? keypress? neither? #238

Open
dbates-wk opened this issue Jul 8, 2019 · 7 comments

Comments

@dbates-wk
Copy link

dbates-wk commented Jul 8, 2019

Create a page with the following markup:

<!DOCTYPE html>
<html>
<body>
<p>Focus the text field below. Then press a key on the keyboard. The output will be the value of the field seen one event loop turn after the listed event dispatched.</p>
<input id="input">
<pre id="console"></pre>
<script>
// Note that scheduling a timer from an Input event handler is for aesthetics only: to make the
// logged Input event be ordered like the spec'ed DOM dispatch event order. By the time the Input
// event fires the DOM is guaranteed to have been updated. So, no timer is needed.
let input = document.getElementById("input");
for (let eventName of ["keydown", "keypress", "keyup", "input"]) {
    input.addEventListener(eventName, (event) => {
        window.setTimeout(() => {
            log(`${eventName}: ${input.value}`);
        }, 0);
    });
}

function log(message)
{
    document.getElementById("console").appendChild(document.createTextNode(message + "\n"));
}
</script>
</body>
</html>

Open ^^^ in your browser and follow the instructions. I'm seeing different things in different browsers™.

In Mac Firefox 67.0.4 I see, emphasis mine:

keydown: 
keypress: a
input: a
keyup: a

On iOS, I see the following (with auto capitalization is off under Settings > General > Keyboard > Hardware Keyboard), emphasis mine:

keydown: 
keypress: 
input: a
keyup: a

In Mac Safari and Chrome Canary for Mac version 77.0.3847.0, emphasis mine:

keydown: a
keypress: a
input: a
keyup: a

For the last two browsers above what is happening is that text insertion (i.e. a DOM update) is occurring within the same event loop as the the keydown.

Spec makes no mention that DOM update must occur in same event loop as keydown or keypress (that's what Firefox is doing). Spec only guarantees DOM is updated when input event is dispatched.

Real-world examples

I know of two sites that assume DOM is updated on keypress:

  1. Google Sheets

Repro steps: Create a new sheet. Tap on a cell. Type into it. Formula bar is updated with DOM value of cell on timer scheduled from keypress (read: not input event).

  1. Bitbucket Server (formerly known as Stash)

Repro steps: In a pull request comment, type @. Bitbucket schedules a timer on keypress and expects text insertion (i.e DOM update) when timer fires so that when it reads <textarea>'s value it will see the typed @ and trigger its suggestion UI.

What to do?

What is the expected behavior? Should spec require DOM be updated for text insertion/deletion in same event loop iteration that keydown is dispatched? same event loop iteration that keypress is dispatched? Should we keep the current spec text with no such requirement? Should we spec out explicitly that there is no such requirement?

@dbates-wk
Copy link
Author

@dbates-wk
Copy link
Author

@annevk

@dbates-wk
Copy link
Author

Update: Mac Firefox 67.0.4 is non-deterministic.

Sometimes it is, emphasis mine:

keydown: 
keypress: a
input: a
keyup: a

and sometimes it is:

keydown: a
keypress: a
input: a
keyup: a

@annevk
Copy link
Member

annevk commented Jul 10, 2019

I do think we should spec this. In particular, there's some kind of abstract "key down operation" that user agents have to support which at some point queues a task to do various things (e.g., dispatching the event, handling whether it was canceled if appropriate?, mutating the DOM?, etc.).

See also #142.

(Think of "key down operation" as an abstract representation of the input you get from the OS / "kernel". Perhaps that's not actually "key down" in which case it should be adjusted somewhat. I haven't looked into this recently.)

caiolima pushed a commit to caiolima/webkit that referenced this issue Jul 16, 2019
https://bugs.webkit.org/show_bug.cgi?id=199587
<rdar://problem/51616845>

Reviewed by Brent Fulgham.

Source/WebCore:

Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
into a holding tank. The timers continue to tick, but are barred from executing their action until
the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
allocate a holding tank once per document, only if the quirk is active, and this allocation is done
when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
of the document.

The story behind the quirk:

On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
event. It could happen in the same event loop iteration as the key press (as Google expects), the
next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
functionality was available via onpropertychange in IE < 9).

See also <w3c/uievents#238>, which is tracking a spec. text update for
this quirk.

Test: fast/events/ios/dom-update-on-keydown-quirk.html

[1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)

* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
Add some files to the project.

* dom/Document.cpp:
(WebCore::Document::domTimerHoldingTank): Added.
* dom/Document.h:
(WebCore::Document::domTimerHoldingTankIfExists): Added.

* page/DOMTimer.cpp:
(WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
(WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
(WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
we do not suspend timers in the holding tank is because:
    1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
    Though smart supension logic could avoid this. See (3).

    2. Empirical observations indicate that the keyboard will perform the insertion or deletion
    reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
    So, the timers in the holding tank are short-lived.

    3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
    suspension reasons (timers currently can only have one suspension reason) or alternatively defer
    scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
* page/EventHandler.cpp:
(WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
scheduled on keydown and keypress into the holding tank if the quirk is enabled.
* page/Quirks.cpp:
(WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
* page/Quirks.h:
* page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
also lets us enable the quirk for all sites or for certain third-party apps if desired.
* page/ios/DOMTimerHoldingTank.cpp: Added.
(WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
(WebCore::DOMTimerHoldingTank::add):
(WebCore::DOMTimerHoldingTank::remove):
(WebCore::DOMTimerHoldingTank::contains):
(WebCore::DOMTimerHoldingTank::removeAll):
(WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
* page/ios/DOMTimerHoldingTank.h: Added.
(WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::isDeferring):

Source/WebKit:

Remove all timers from the holding tank on text insertion or deletion (represented as an
editing command). Timers that were in the holding tank never stopped ticking and will now
be able to execute their action.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::executeEditingCommand):
(WebKit::WebPage::insertTextAsync):
(WebKit::WebPage::setCompositionAsync):
(WebKit::WebPage::confirmCompositionAsync):
Call platformWillPerformEditingCommand().

* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::platformWillPerformEditingCommand): Added.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
tank if we have a holding tank.

LayoutTests:

Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
zero timer scheduled from keydown, keypress, keyup, and input.

* fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
* fast/events/ios/dom-update-on-keydown-quirk.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@247444 268f45cc-cd09-0410-ab3c-d52691b4dbfc
zdobersek pushed a commit to Igalia/webkit that referenced this issue Jul 20, 2019
https://bugs.webkit.org/show_bug.cgi?id=199587
<rdar://problem/51616845>

Reviewed by Brent Fulgham.

Source/WebCore:

Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
into a holding tank. The timers continue to tick, but are barred from executing their action until
the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
allocate a holding tank once per document, only if the quirk is active, and this allocation is done
when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
of the document.

The story behind the quirk:

On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
event. It could happen in the same event loop iteration as the key press (as Google expects), the
next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
functionality was available via onpropertychange in IE < 9).

See also <w3c/uievents#238>, which is tracking a spec. text update for
this quirk.

Test: fast/events/ios/dom-update-on-keydown-quirk.html

[1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)

* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
Add some files to the project.

* dom/Document.cpp:
(WebCore::Document::domTimerHoldingTank): Added.
* dom/Document.h:
(WebCore::Document::domTimerHoldingTankIfExists): Added.

* page/DOMTimer.cpp:
(WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
(WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
(WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
we do not suspend timers in the holding tank is because:
    1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
    Though smart supension logic could avoid this. See (3).

    2. Empirical observations indicate that the keyboard will perform the insertion or deletion
    reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
    So, the timers in the holding tank are short-lived.

    3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
    suspension reasons (timers currently can only have one suspension reason) or alternatively defer
    scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
* page/EventHandler.cpp:
(WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
scheduled on keydown and keypress into the holding tank if the quirk is enabled.
* page/Quirks.cpp:
(WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
* page/Quirks.h:
* page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
also lets us enable the quirk for all sites or for certain third-party apps if desired.
* page/ios/DOMTimerHoldingTank.cpp: Added.
(WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
(WebCore::DOMTimerHoldingTank::add):
(WebCore::DOMTimerHoldingTank::remove):
(WebCore::DOMTimerHoldingTank::contains):
(WebCore::DOMTimerHoldingTank::removeAll):
(WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
* page/ios/DOMTimerHoldingTank.h: Added.
(WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::isDeferring):

Source/WebKit:

Remove all timers from the holding tank on text insertion or deletion (represented as an
editing command). Timers that were in the holding tank never stopped ticking and will now
be able to execute their action.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::executeEditingCommand):
(WebKit::WebPage::insertTextAsync):
(WebKit::WebPage::setCompositionAsync):
(WebKit::WebPage::confirmCompositionAsync):
Call platformWillPerformEditingCommand().

* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::platformWillPerformEditingCommand): Added.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
tank if we have a holding tank.

LayoutTests:

Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
zero timer scheduled from keydown, keypress, keyup, and input.

* fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
* fast/events/ios/dom-update-on-keydown-quirk.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@247530 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ashkulz pushed a commit to qtwebkit/webkit-mirror that referenced this issue Jul 22, 2019
    Typing into a cell in a Google Sheet lags behind by one character
    https://bugs.webkit.org/show_bug.cgi?id=199587
    <rdar://problem/51616845>

    Reviewed by Brent Fulgham.

    Source/WebCore:

    Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
    into a holding tank. The timers continue to tick, but are barred from executing their action until
    the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
    allocate a holding tank once per document, only if the quirk is active, and this allocation is done
    when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
    of the document.

    The story behind the quirk:

    On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
    will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
    UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
    event. It could happen in the same event loop iteration as the key press (as Google expects), the
    next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
    a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
    Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
    functionality was available via onpropertychange in IE < 9).

    See also <w3c/uievents#238>, which is tracking a spec. text update for
    this quirk.

    Test: fast/events/ios/dom-update-on-keydown-quirk.html

    [1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)

    * SourcesCocoa.txt:
    * WebCore.xcodeproj/project.pbxproj:
    Add some files to the project.

    * dom/Document.cpp:
    (WebCore::Document::domTimerHoldingTank): Added.
    * dom/Document.h:
    (WebCore::Document::domTimerHoldingTankIfExists): Added.

    * page/DOMTimer.cpp:
    (WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
    (WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
    (WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
    shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
    let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
    we do not suspend timers in the holding tank is because:
        1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
        Though smart supension logic could avoid this. See (3).

        2. Empirical observations indicate that the keyboard will perform the insertion or deletion
        reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
        So, the timers in the holding tank are short-lived.

        3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
        suspension reasons (timers currently can only have one suspension reason) or alternatively defer
        scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
    * page/EventHandler.cpp:
    (WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
    scheduled on keydown and keypress into the holding tank if the quirk is enabled.
    * page/Quirks.cpp:
    (WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
    * page/Quirks.h:
    * page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
    also lets us enable the quirk for all sites or for certain third-party apps if desired.
    * page/ios/DOMTimerHoldingTank.cpp: Added.
    (WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
    (WebCore::DOMTimerHoldingTank::add):
    (WebCore::DOMTimerHoldingTank::remove):
    (WebCore::DOMTimerHoldingTank::contains):
    (WebCore::DOMTimerHoldingTank::removeAll):
    (WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
    * page/ios/DOMTimerHoldingTank.h: Added.
    (WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
    (WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
    (WebCore::DeferDOMTimersForScope::isDeferring):

    Source/WebKit:

    Remove all timers from the holding tank on text insertion or deletion (represented as an
    editing command). Timers that were in the holding tank never stopped ticking and will now
    be able to execute their action.

    * WebProcess/WebPage/WebPage.cpp:
    (WebKit::WebPage::executeEditingCommand):
    (WebKit::WebPage::insertTextAsync):
    (WebKit::WebPage::setCompositionAsync):
    (WebKit::WebPage::confirmCompositionAsync):
    Call platformWillPerformEditingCommand().

    * WebProcess/WebPage/WebPage.h:
    (WebKit::WebPage::platformWillPerformEditingCommand): Added.
    * WebProcess/WebPage/ios/WebPageIOS.mm:
    (WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
    tank if we have a holding tank.

    LayoutTests:

    Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
    zero timer scheduled from keydown, keypress, keyup, and input.

    * fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
    * fast/events/ios/dom-update-on-keydown-quirk.html: Added.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247530 268f45cc-cd09-0410-ab3c-d52691b4dbfc

git-svn-id: http://svn.webkit.org/repository/webkit/branches/safari-608-branch@247606 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@garykac
Copy link
Member

garykac commented Jul 31, 2020

I started working on a more formal algorithm for dispatching keyboard/input/composition events. It's in the early stages and is far from complete, but my goal is to be able to specify these behaviors enough to address your concerns.

https://docs.google.com/document/d/1LJQvjEmWZGzVgZnofpvdkxMj1hEnLniD72XD4DLJWx4/edit?usp=sharing

Note that we won't be defining any behaviors relative to keypress because that event has been deprecated.

@annevk
Copy link
Member

annevk commented Aug 4, 2020

Is it realistic to expect that it can be removed?

@garykac
Copy link
Member

garykac commented Aug 4, 2020

Hmm.. I guess I was answering the question from the title "should input be in same event loop as keypress" with "we could define input in same loop as keydown; and keypress in same loop as keydown" (or something like that) so that the definitions are relative to non-deprecated events.

But once there's a proper algorithm, then it doesn't make sense to think of it that way. Since it would be written more like:

  • fire keydown in event loop X
  • fire input in event loop X
  • fire keypress in event loop X

Sorry about the confusion there. To more directly answer your question, we can't remove keypress and it needs to be included in the spec, I just want to ensure that it can (in theory) be removed without breaking the spec.

ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=199587
<rdar://problem/51616845>

Reviewed by Brent Fulgham.

Source/WebCore:

Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
into a holding tank. The timers continue to tick, but are barred from executing their action until
the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
allocate a holding tank once per document, only if the quirk is active, and this allocation is done
when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
of the document.

The story behind the quirk:

On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
event. It could happen in the same event loop iteration as the key press (as Google expects), the
next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
functionality was available via onpropertychange in IE < 9).

See also <w3c/uievents#238>, which is tracking a spec. text update for
this quirk.

Test: fast/events/ios/dom-update-on-keydown-quirk.html

[1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)

* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
Add some files to the project.

* dom/Document.cpp:
(WebCore::Document::domTimerHoldingTank): Added.
* dom/Document.h:
(WebCore::Document::domTimerHoldingTankIfExists): Added.

* page/DOMTimer.cpp:
(WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
(WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
(WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
we do not suspend timers in the holding tank is because:
    1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
    Though smart supension logic could avoid this. See (3).

    2. Empirical observations indicate that the keyboard will perform the insertion or deletion
    reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
    So, the timers in the holding tank are short-lived.

    3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
    suspension reasons (timers currently can only have one suspension reason) or alternatively defer
    scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
* page/EventHandler.cpp:
(WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
scheduled on keydown and keypress into the holding tank if the quirk is enabled.
* page/Quirks.cpp:
(WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
* page/Quirks.h:
* page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
also lets us enable the quirk for all sites or for certain third-party apps if desired.
* page/ios/DOMTimerHoldingTank.cpp: Added.
(WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
(WebCore::DOMTimerHoldingTank::add):
(WebCore::DOMTimerHoldingTank::remove):
(WebCore::DOMTimerHoldingTank::contains):
(WebCore::DOMTimerHoldingTank::removeAll):
(WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
* page/ios/DOMTimerHoldingTank.h: Added.
(WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::isDeferring):

Source/WebKit:

Remove all timers from the holding tank on text insertion or deletion (represented as an
editing command). Timers that were in the holding tank never stopped ticking and will now
be able to execute their action.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::executeEditingCommand):
(WebKit::WebPage::insertTextAsync):
(WebKit::WebPage::setCompositionAsync):
(WebKit::WebPage::confirmCompositionAsync):
Call platformWillPerformEditingCommand().

* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::platformWillPerformEditingCommand): Added.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
tank if we have a holding tank.

LayoutTests:

Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
zero timer scheduled from keydown, keypress, keyup, and input.

* fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
* fast/events/ios/dom-update-on-keydown-quirk.html: Added.

Canonical link: https://commits.webkit.org/213676@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247444 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ryanhaddad pushed a commit to WebKit/WebKit that referenced this issue Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=199587
<rdar://problem/51616845>

Reviewed by Brent Fulgham.

Source/WebCore:

Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
into a holding tank. The timers continue to tick, but are barred from executing their action until
the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
allocate a holding tank once per document, only if the quirk is active, and this allocation is done
when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
of the document.

The story behind the quirk:

On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
event. It could happen in the same event loop iteration as the key press (as Google expects), the
next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
functionality was available via onpropertychange in IE < 9).

See also <w3c/uievents#238>, which is tracking a spec. text update for
this quirk.

Test: fast/events/ios/dom-update-on-keydown-quirk.html

[1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)

* SourcesCocoa.txt:
* WebCore.xcodeproj/project.pbxproj:
Add some files to the project.

* dom/Document.cpp:
(WebCore::Document::domTimerHoldingTank): Added.
* dom/Document.h:
(WebCore::Document::domTimerHoldingTankIfExists): Added.

* page/DOMTimer.cpp:
(WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
(WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
(WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
we do not suspend timers in the holding tank is because:
    1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
    Though smart supension logic could avoid this. See (3).

    2. Empirical observations indicate that the keyboard will perform the insertion or deletion
    reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
    So, the timers in the holding tank are short-lived.

    3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
    suspension reasons (timers currently can only have one suspension reason) or alternatively defer
    scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
* page/EventHandler.cpp:
(WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
scheduled on keydown and keypress into the holding tank if the quirk is enabled.
* page/Quirks.cpp:
(WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
* page/Quirks.h:
* page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
also lets us enable the quirk for all sites or for certain third-party apps if desired.
* page/ios/DOMTimerHoldingTank.cpp: Added.
(WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
(WebCore::DOMTimerHoldingTank::add):
(WebCore::DOMTimerHoldingTank::remove):
(WebCore::DOMTimerHoldingTank::contains):
(WebCore::DOMTimerHoldingTank::removeAll):
(WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
* page/ios/DOMTimerHoldingTank.h: Added.
(WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
(WebCore::DeferDOMTimersForScope::isDeferring):

Source/WebKit:

Remove all timers from the holding tank on text insertion or deletion (represented as an
editing command). Timers that were in the holding tank never stopped ticking and will now
be able to execute their action.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::executeEditingCommand):
(WebKit::WebPage::insertTextAsync):
(WebKit::WebPage::setCompositionAsync):
(WebKit::WebPage::confirmCompositionAsync):
Call platformWillPerformEditingCommand().

* WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::platformWillPerformEditingCommand): Added.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
tank if we have a holding tank.

LayoutTests:

Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
zero timer scheduled from keydown, keypress, keyup, and input.

* fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
* fast/events/ios/dom-update-on-keydown-quirk.html: Added.

Canonical link: https://commits.webkit.org/213750@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247530 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JonWBedard pushed a commit to WebKit/WebKit that referenced this issue Jan 6, 2021
    Typing into a cell in a Google Sheet lags behind by one character
    https://bugs.webkit.org/show_bug.cgi?id=199587
    <rdar://problem/51616845>

    Reviewed by Brent Fulgham.

    Source/WebCore:

    Add a Google Sheets quirk. Put all DOM timers scheduled from keydown and keypress event listeners
    into a holding tank. The timers continue to tick, but are barred from executing their action until
    the next text insertion or deletion or 32 ms (on device) have elapsed, whichever is sooner. We only
    allocate a holding tank once per document, only if the quirk is active, and this allocation is done
    when the document schedules a timer on keydown or keypress. The holding tank lives for the lifetime
    of the document.

    The story behind the quirk:

    On keypress Google Sheets schedules timers and expects that a DOM update will occur (i.e. text
    will be inserted or deleted) within the same event loop iteration as the dispatched keypress. The
    UI Events spec. [1] makes no such guarantee of when a DOM update must occur in relation to the keypress
    event. It could happen in the same event loop iteration as the key press (as Google expects), the
    next iteration, 500ms later, 2 minutes later, etc. What the spec does guarantee is that by the time
    a DOM input event is dispatched that the DOM will be updated. And this is the solution to the problem
    Google Sheets is trying to solve, but is doing so using pre-IE 9 technology (though similar
    functionality was available via onpropertychange in IE < 9).

    See also <w3c/uievents#238>, which is tracking a spec. text update for
    this quirk.

    Test: fast/events/ios/dom-update-on-keydown-quirk.html

    [1] <https://w3c.github.io/uievents/> (Editor's Draft, 14 October 2018)

    * SourcesCocoa.txt:
    * WebCore.xcodeproj/project.pbxproj:
    Add some files to the project.

    * dom/Document.cpp:
    (WebCore::Document::domTimerHoldingTank): Added.
    * dom/Document.h:
    (WebCore::Document::domTimerHoldingTankIfExists): Added.

    * page/DOMTimer.cpp:
    (WebCore::DOMTimer::install): Put the newly instantiated timer into the holding tank.
    (WebCore::DOMTimer::removeById): Remove the timer from the holding tank.
    (WebCore::DOMTimer::fired): Check if the timer is in the holding tank. If it is and it is a one-
    shot timer then schedule it for the next event loop iteration. If it's a repeating timer just
    let it continue ticking. Otherwise, do what we no now and execute the timer's action. The reason
    we do not suspend timers in the holding tank is because:
        1. Far out timers (Google Sheets registers timers as far out as 5 minutes!) are not penalized.
        Though smart supension logic could avoid this. See (3).

        2. Empirical observations indicate that the keyboard will perform the insertion or deletion
        reasonably quickly (not the same event loop iteration as the keydown, but within two iterations out).
        So, the timers in the holding tank are short-lived.

        3. Simplifies the code. There is no need to keep additional bookkeeping to track multiple timer
        suspension reasons (timers currently can only have one suspension reason) or alternatively defer
        scheduling a timer until a later time and computing a new "fair" firing time when scheduled.
    * page/EventHandler.cpp:
    (WebCore::EventHandler::internalKeyEvent): Place a token on the stack to put all DOM timers
    scheduled on keydown and keypress into the holding tank if the quirk is enabled.
    * page/Quirks.cpp:
    (WebCore::Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand const): Added.
    * page/Quirks.h:
    * page/Settings.yaml: Added setting so that this quirk can be enabled from a layout test. This setting
    also lets us enable the quirk for all sites or for certain third-party apps if desired.
    * page/ios/DOMTimerHoldingTank.cpp: Added.
    (WebCore::DOMTimerHoldingTank::DOMTimerHoldingTank):
    (WebCore::DOMTimerHoldingTank::add):
    (WebCore::DOMTimerHoldingTank::remove):
    (WebCore::DOMTimerHoldingTank::contains):
    (WebCore::DOMTimerHoldingTank::removeAll):
    (WebCore::DOMTimerHoldingTank::stopExceededMaximumHoldTimer):
    * page/ios/DOMTimerHoldingTank.h: Added.
    (WebCore::DeferDOMTimersForScope::DeferDOMTimersForScope):
    (WebCore::DeferDOMTimersForScope::~DeferDOMTimersForScope):
    (WebCore::DeferDOMTimersForScope::isDeferring):

    Source/WebKit:

    Remove all timers from the holding tank on text insertion or deletion (represented as an
    editing command). Timers that were in the holding tank never stopped ticking and will now
    be able to execute their action.

    * WebProcess/WebPage/WebPage.cpp:
    (WebKit::WebPage::executeEditingCommand):
    (WebKit::WebPage::insertTextAsync):
    (WebKit::WebPage::setCompositionAsync):
    (WebKit::WebPage::confirmCompositionAsync):
    Call platformWillPerformEditingCommand().

    * WebProcess/WebPage/WebPage.h:
    (WebKit::WebPage::platformWillPerformEditingCommand): Added.
    * WebProcess/WebPage/ios/WebPageIOS.mm:
    (WebKit::WebPage::platformWillPerformEditingCommand): Remove all the timers from the holding
    tank if we have a holding tank.

    LayoutTests:

    Add a test that enables the quirk and ensures that the DOM is up-to-date on expiration of a
    zero timer scheduled from keydown, keypress, keyup, and input.

    * fast/events/ios/dom-update-on-keydown-quirk-expected.txt: Added.
    * fast/events/ios/dom-update-on-keydown-quirk.html: Added.

    Canonical link: https://commits.webkit.org/213750@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247530 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/213666.42@safari-608-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-608-branch@247606 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants