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 <https://github.com/w3c/uievents/issues/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
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 18db31c..ad265ff 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2019-07-15 Daniel Bates <dabates@apple.com>
+
+ 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.
+
+ 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.
+
2019-07-15 Ryan Haddad <ryanhaddad@apple.com>
Unreviewed, rolling out r247397, r247378, r247366.
diff --git a/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk-expected.txt b/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk-expected.txt
new file mode 100644
index 0000000..fad26c2
--- /dev/null
+++ b/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk-expected.txt
@@ -0,0 +1,21 @@
+This tests that the value of the field is updated by the time any timer scheduled on keydown, keypress, or keyup fires. To run this test manually, focus the text field and press [.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+
+For keydown:
+PASS document.getElementById("input").value is "["
+
+For keypress:
+PASS document.getElementById("input").value is "["
+
+For input:
+PASS document.getElementById("input").value is "["
+
+For keyup:
+PASS document.getElementById("input").value is "["
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk.html b/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk.html
new file mode 100644
index 0000000..38e22f7
--- /dev/null
+++ b/LayoutTests/fast/events/ios/dom-update-on-keydown-quirk.html
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+</head>
+<body>
+<input id="input">
+<script>
+const expectedValue = "["; // [ chosen to avoid having to handle or disable iOS auto capitalization.
+
+let input = document.getElementById("input");
+
+function done()
+{
+ document.body.removeChild(input);
+ finishJSTest();
+}
+
+async function runTest()
+{
+ if (window.internals.settings)
+ internals.settings.setNeedsDeferKeyDownAndKeyPressTimersUntilNextEditingCommandQuirk(true);
+
+ if (window.testRunner)
+ await UIHelper.callFunctionAndWaitForEvent(() => UIHelper.activateElement(input), input, "focus");
+
+ function checkResult(event) {
+ // 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.
+ window.setTimeout(() => {
+ debug(`<br>For ${event.type}:`);
+ shouldBeEqualToString('document.getElementById("input").value', expectedValue);
+ if (event.type === "keyup")
+ done();
+ }, 0);
+ }
+
+ for (let eventName of ["keydown", "keypress", "keyup", "input"])
+ input.addEventListener(eventName, checkResult, { once: true });
+
+ if (window.testRunner)
+ UIHelper.keyDown(expectedValue);
+}
+
+window.jsTestIsAsync = true;
+description("This tests that the value of the field is updated by the time any timer scheduled on keydown, keypress, or keyup fires. To run this test manually, focus the text field and press <kbd>[</kbd>.");
+
+runTest();
+</script>
+</body>
+</html>
\ No newline at end of file
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 4ad89de..9903bb46 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,82 @@
+2019-07-15 Daniel Bates <dabates@apple.com>
+
+ 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.
+
+ 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 <https://github.com/w3c/uievents/issues/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):
+
2019-07-15 Ryan Haddad <ryanhaddad@apple.com>
Unreviewed, rolling out r247397, r247378, r247366.
diff --git a/Source/WebCore/SourcesCocoa.txt b/Source/WebCore/SourcesCocoa.txt
index db8aa2e..e83348a 100644
--- a/Source/WebCore/SourcesCocoa.txt
+++ b/Source/WebCore/SourcesCocoa.txt
@@ -131,6 +131,7 @@
page/cocoa/SettingsBaseCocoa.mm
page/ios/ContentChangeObserver.cpp
+page/ios/DOMTimerHoldingTank.cpp
page/ios/EventHandlerIOS.mm
page/ios/FrameIOS.mm
page/ios/WebEventRegion.mm
diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
index 9a8ef93..df2069e 100644
--- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj
+++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
@@ -4191,6 +4191,7 @@
CE057FA61220731100A476D5 /* DocumentMarkerController.h in Headers */ = {isa = PBXBuildFile; fileRef = CE057FA41220731100A476D5 /* DocumentMarkerController.h */; settings = {ATTRIBUTES = (Private, ); }; };
CE08C3D2152B599A0021B8C2 /* AlternativeTextController.h in Headers */ = {isa = PBXBuildFile; fileRef = CE08C3D0152B599A0021B8C2 /* AlternativeTextController.h */; settings = {ATTRIBUTES = (); }; };
CE1866451F72E5B400A0CAB6 /* MarkedText.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1866431F72E5B400A0CAB6 /* MarkedText.h */; settings = {ATTRIBUTES = (Private, ); }; };
+ CE1A501F22D5350900CBC927 /* DOMTimerHoldingTank.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1A501D22D5350900CBC927 /* DOMTimerHoldingTank.h */; settings = {ATTRIBUTES = (Private, ); }; };
CE2849871CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h in Headers */ = {isa = PBXBuildFile; fileRef = CE2849861CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h */; };
CE5169E721F1B84700EA4F78 /* ColorIOS.h in Headers */ = {isa = PBXBuildFile; fileRef = CE5169E521F1B84700EA4F78 /* ColorIOS.h */; settings = {ATTRIBUTES = (Private, ); }; };
CE5FA255209E48C50051D700 /* ContentSecurityPolicyClient.h in Headers */ = {isa = PBXBuildFile; fileRef = CE5FA253209E48C50051D700 /* ContentSecurityPolicyClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -13857,6 +13858,8 @@
CE08C3D0152B599A0021B8C2 /* AlternativeTextController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AlternativeTextController.h; sourceTree = "<group>"; };
CE1866421F72E5B400A0CAB6 /* MarkedText.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = MarkedText.cpp; sourceTree = "<group>"; };
CE1866431F72E5B400A0CAB6 /* MarkedText.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MarkedText.h; sourceTree = "<group>"; };
+ CE1A501D22D5350900CBC927 /* DOMTimerHoldingTank.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DOMTimerHoldingTank.h; sourceTree = "<group>"; };
+ CE1A501E22D5350900CBC927 /* DOMTimerHoldingTank.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = DOMTimerHoldingTank.cpp; sourceTree = "<group>"; };
CE2849861CA360DF00B4A57F /* ContentSecurityPolicyDirectiveNames.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ContentSecurityPolicyDirectiveNames.h; path = csp/ContentSecurityPolicyDirectiveNames.h; sourceTree = "<group>"; };
CE2849881CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ContentSecurityPolicyDirectiveNames.cpp; path = csp/ContentSecurityPolicyDirectiveNames.cpp; sourceTree = "<group>"; };
CE5169E521F1B84700EA4F78 /* ColorIOS.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ColorIOS.h; sourceTree = "<group>"; };
@@ -16576,6 +16579,8 @@
children = (
6FE9F09222211035004C5082 /* ContentChangeObserver.cpp */,
6FB5E212221F2447003989CF /* ContentChangeObserver.h */,
+ CE1A501E22D5350900CBC927 /* DOMTimerHoldingTank.cpp */,
+ CE1A501D22D5350900CBC927 /* DOMTimerHoldingTank.h */,
FE6938B51045D67E008EABB6 /* EventHandlerIOS.mm */,
FED13D3B0CEA936A00D89466 /* FrameIOS.mm */,
225A16B30D5C11E900090295 /* WebEventRegion.h */,
@@ -29020,6 +29025,7 @@
C544274B11A57E7A0063A749 /* DOMStringList.h in Headers */,
BC64640A11D7F304006455B0 /* DOMStringMap.h in Headers */,
188604B40F2E654A000B6443 /* DOMTimer.h in Headers */,
+ CE1A501F22D5350900CBC927 /* DOMTimerHoldingTank.h in Headers */,
05FD69E012845D4300B2BEB3 /* DOMTimeStamp.h in Headers */,
76FC2B0C12370DA0006A991A /* DOMTokenList.h in Headers */,
2E37DFDB12DBAFB800A6B233 /* DOMURL.h in Headers */,
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 83bafc9..1035fb3 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -262,6 +262,7 @@
#if PLATFORM(IOS_FAMILY)
#include "ContentChangeObserver.h"
#include "CSSFontSelector.h"
+#include "DOMTimerHoldingTank.h"
#include "DeviceMotionClientIOS.h"
#include "DeviceMotionController.h"
#include "DeviceOrientationClientIOS.h"
@@ -8186,12 +8187,22 @@
#endif
#if PLATFORM(IOS_FAMILY)
+
ContentChangeObserver& Document::contentChangeObserver()
{
if (!m_contentChangeObserver)
m_contentChangeObserver = std::make_unique<ContentChangeObserver>(*this);
return *m_contentChangeObserver;
}
+
+DOMTimerHoldingTank& Document::domTimerHoldingTank()
+{
+ if (m_domTimerHoldingTank)
+ return *m_domTimerHoldingTank;
+ m_domTimerHoldingTank = std::make_unique<DOMTimerHoldingTank>();
+ return *m_domTimerHoldingTank;
+}
+
#endif
bool Document::hasEvaluatedUserAgentScripts() const
diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
index a022efd..34913c5 100644
--- a/Source/WebCore/dom/Document.h
+++ b/Source/WebCore/dom/Document.h
@@ -106,6 +106,7 @@
class ContentChangeObserver;
class DOMImplementation;
class DOMSelection;
+class DOMTimerHoldingTank;
class DOMWindow;
class DOMWrapperWorld;
class Database;
@@ -883,6 +884,9 @@
void processWebAppOrientations();
WEBCORE_EXPORT ContentChangeObserver& contentChangeObserver();
+
+ DOMTimerHoldingTank* domTimerHoldingTankIfExists() { return m_domTimerHoldingTank.get(); }
+ DOMTimerHoldingTank& domTimerHoldingTank();
#endif
void processViewport(const String& features, ViewportArguments::Type origin);
@@ -2044,6 +2048,7 @@
Ref<UndoManager> m_undoManager;
#if PLATFORM(IOS_FAMILY)
std::unique_ptr<ContentChangeObserver> m_contentChangeObserver;
+ std::unique_ptr<DOMTimerHoldingTank> m_domTimerHoldingTank;
#endif
HashMap<Element*, ElementIdentifier> m_identifiedElementsMap;
diff --git a/Source/WebCore/page/DOMTimer.cpp b/Source/WebCore/page/DOMTimer.cpp
index 6a22dbe..7f57537 100644
--- a/Source/WebCore/page/DOMTimer.cpp
+++ b/Source/WebCore/page/DOMTimer.cpp
@@ -43,6 +43,7 @@
#if PLATFORM(IOS_FAMILY)
#include "ContentChangeObserver.h"
+#include "DOMTimerHoldingTank.h"
#endif
namespace WebCore {
@@ -196,8 +197,12 @@
if (NestedTimersMap* nestedTimers = NestedTimersMap::instanceForContext(context))
nestedTimers->add(timer->m_timeoutId, *timer);
#if PLATFORM(IOS_FAMILY)
- if (is<Document>(context))
- downcast<Document>(context).contentChangeObserver().didInstallDOMTimer(*timer, timeout, singleShot);
+ if (is<Document>(context)) {
+ auto& document = downcast<Document>(context);
+ document.contentChangeObserver().didInstallDOMTimer(*timer, timeout, singleShot);
+ if (DeferDOMTimersForScope::isDeferring())
+ document.domTimerHoldingTank().add(*timer);
+ }
#endif
return timer->m_timeoutId;
}
@@ -213,8 +218,11 @@
#if PLATFORM(IOS_FAMILY)
if (is<Document>(context)) {
auto& document = downcast<Document>(context);
- if (auto* timer = document.findTimeout(timeoutId))
+ if (auto* timer = document.findTimeout(timeoutId)) {
document.contentChangeObserver().didRemoveDOMTimer(*timer);
+ if (auto* holdingTank = document.domTimerHoldingTankIfExists())
+ holdingTank->remove(*timer);
+ }
}
#endif
@@ -284,6 +292,17 @@
ASSERT(scriptExecutionContext());
ScriptExecutionContext& context = *scriptExecutionContext();
+#if PLATFORM(IOS_FAMILY)
+ if (is<Document>(context)) {
+ auto& document = downcast<Document>(context);
+ if (auto* holdingTank = document.domTimerHoldingTankIfExists(); holdingTank && holdingTank->contains(*this)) {
+ if (!repeatInterval())
+ startOneShot(0_s);
+ return;
+ }
+ }
+#endif
+
DOMTimerFireState fireState(context, std::min(m_nestingLevel + 1, maxTimerNestingLevel));
if (m_userGestureTokenToForward && m_userGestureTokenToForward->hasExpired(maxIntervalForUserGestureForwarding))
diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
index ef05ff1..b97a8d5 100644
--- a/Source/WebCore/page/EventHandler.cpp
+++ b/Source/WebCore/page/EventHandler.cpp
@@ -131,6 +131,10 @@
#include "RuntimeEnabledFeatures.h"
#endif
+#if PLATFORM(IOS_FAMILY)
+#include "DOMTimerHoldingTank.h"
+#endif
+
namespace WebCore {
using namespace HTMLNames;
@@ -3341,6 +3345,10 @@
if (accessibilityPreventsEventPropagation(keydown))
keydown->stopPropagation();
+#if PLATFORM(IOS_FAMILY)
+ DeferDOMTimersForScope deferralScope { m_frame.document()->quirks().needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() };
+#endif
+
element->dispatchEvent(keydown);
if (handledByInputMethod)
return true;
diff --git a/Source/WebCore/page/Quirks.cpp b/Source/WebCore/page/Quirks.cpp
index bbae28d..8b58b09 100644
--- a/Source/WebCore/page/Quirks.cpp
+++ b/Source/WebCore/page/Quirks.cpp
@@ -348,6 +348,22 @@
return false;
}
+bool Quirks::needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() const
+{
+#if PLATFORM(IOS_FAMILY)
+ if (m_document->settings().needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommandQuirk())
+ return true;
+
+ if (!needsQuirks())
+ return false;
+
+ auto& url = m_document->topDocument().url();
+ return equalLettersIgnoringASCIICase(url.host(), "docs.google.com") && url.path().startsWithIgnoringASCIICase("/spreadsheets/");
+#else
+ return false;
+#endif
+}
+
// FIXME(<rdar://problem/50394969>): Remove after desmos.com adopts inputmode="none".
bool Quirks::needsInputModeNoneImplicitly(const HTMLElement& element) const
{
diff --git a/Source/WebCore/page/Quirks.h b/Source/WebCore/page/Quirks.h
index 4cb44c2..44092d2 100644
--- a/Source/WebCore/page/Quirks.h
+++ b/Source/WebCore/page/Quirks.h
@@ -56,6 +56,7 @@
#endif
bool shouldDisablePointerEventsQuirk() const;
bool needsInputModeNoneImplicitly(const HTMLElement&) const;
+ bool needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommand() const;
WEBCORE_EXPORT bool shouldDispatchSyntheticMouseEventsWhenModifyingSelection() const;
WEBCORE_EXPORT bool shouldSuppressAutocorrectionAndAutocaptializationInHiddenEditableAreas() const;
diff --git a/Source/WebCore/page/Settings.yaml b/Source/WebCore/page/Settings.yaml
index e097408..7e7d01a 100644
--- a/Source/WebCore/page/Settings.yaml
+++ b/Source/WebCore/page/Settings.yaml
@@ -162,6 +162,15 @@
needsKeyboardEventDisambiguationQuirks:
initial: false
+# This is an iOS-specific quirk. Unlike Mac, keyboard operations are asynchronous and hence a DOM update as
+# a result of text insertion or deletion does not occur within the same event loop iteration as a dispatched
+# DOM keydown event. Some sites, notably Google Sheets, schedule timers on keypress and expect on a DOM update
+# to have occurred on expiration. When enabled, this quirk puts all such scheduled timers in a holding tank
+# until the keyboard performs the insertion or deletion. This gives Google Sheets the illusion that the DOM
+# update happened within the same event loop iteration that the keypress event was dispatched in.
+needsDeferKeyDownAndKeyPressTimersUntilNextEditingCommandQuirk:
+ initial: false
+
treatsAnyTextCSSLinkAsStylesheet:
initial: false
shrinksStandaloneImagesToFit:
diff --git a/Source/WebCore/page/ios/DOMTimerHoldingTank.cpp b/Source/WebCore/page/ios/DOMTimerHoldingTank.cpp
new file mode 100644
index 0000000..3883c3b
--- /dev/null
+++ b/Source/WebCore/page/ios/DOMTimerHoldingTank.cpp
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "DOMTimerHoldingTank.h"
+
+#if PLATFORM(IOS_FAMILY)
+
+#include <wtf/HashSet.h>
+
+namespace WebCore {
+
+#if PLATFORM(IOS_SIMULATOR)
+constexpr Seconds maximumHoldTimeLimit { 50_ms };
+#else
+constexpr Seconds maximumHoldTimeLimit { 32_ms };
+#endif
+
+bool DeferDOMTimersForScope::s_isDeferring { false };
+
+DOMTimerHoldingTank::DOMTimerHoldingTank()
+ : m_exceededMaximumHoldTimer { *this, &DOMTimerHoldingTank::removeAll }
+{
+}
+
+DOMTimerHoldingTank::~DOMTimerHoldingTank() = default;
+
+void DOMTimerHoldingTank::add(const DOMTimer& timer)
+{
+ m_timers.add(&timer);
+ if (!m_exceededMaximumHoldTimer.isActive())
+ m_exceededMaximumHoldTimer.startOneShot(maximumHoldTimeLimit);
+}
+
+void DOMTimerHoldingTank::remove(const DOMTimer& timer)
+{
+ stopExceededMaximumHoldTimer();
+ m_timers.remove(&timer);
+}
+
+bool DOMTimerHoldingTank::contains(const DOMTimer& timer)
+{
+ return m_timers.contains(&timer);
+}
+
+void DOMTimerHoldingTank::removeAll()
+{
+ stopExceededMaximumHoldTimer();
+ m_timers.clear();
+}
+
+inline void DOMTimerHoldingTank::stopExceededMaximumHoldTimer()
+{
+ if (m_exceededMaximumHoldTimer.isActive())
+ m_exceededMaximumHoldTimer.stop();
+}
+
+} // namespace WebCore
+
+#endif // PLATFORM(IOS_FAMILY)
diff --git a/Source/WebCore/page/ios/DOMTimerHoldingTank.h b/Source/WebCore/page/ios/DOMTimerHoldingTank.h
new file mode 100644
index 0000000..60277c7
--- /dev/null
+++ b/Source/WebCore/page/ios/DOMTimerHoldingTank.h
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2019 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#if PLATFORM(IOS_FAMILY)
+
+#include "Timer.h"
+#include <wtf/Forward.h>
+
+namespace WebCore {
+
+class DOMTimer;
+
+class DOMTimerHoldingTank {
+public:
+ DOMTimerHoldingTank();
+ ~DOMTimerHoldingTank();
+
+ void add(const DOMTimer&);
+ void remove(const DOMTimer&);
+ bool contains(const DOMTimer&);
+ WEBCORE_EXPORT void removeAll();
+
+private:
+ void stopExceededMaximumHoldTimer();
+
+ HashSet<const DOMTimer*> m_timers;
+ Timer m_exceededMaximumHoldTimer;
+};
+
+class DeferDOMTimersForScope {
+public:
+ DeferDOMTimersForScope(bool enable)
+ : m_previousIsDeferring { s_isDeferring }
+ {
+ if (enable)
+ s_isDeferring = true;
+ }
+
+ ~DeferDOMTimersForScope() { s_isDeferring = m_previousIsDeferring; }
+
+ static bool isDeferring() { return s_isDeferring; }
+
+private:
+ bool m_previousIsDeferring;
+ static bool s_isDeferring;
+};
+
+} // namespace WebCore
+
+#endif // PLATFORM(IOS_FAMILY)
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index d8f1cbe..d1e1188 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,28 @@
+2019-07-15 Daniel Bates <dabates@apple.com>
+
+ 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.
+
+ 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.
+
2019-07-15 Wenson Hsieh <wenson_hsieh@apple.com>
[macOS 10.15] Cannot unbold selected text when the system font is used
diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
index 06c7316..f1d822b 100644
--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
@@ -1260,7 +1260,9 @@
void WebPage::executeEditingCommand(const String& commandName, const String& argument)
{
- Frame& frame = m_page->focusController().focusedOrMainFrame();
+ platformWillPerformEditingCommand();
+
+ auto& frame = m_page->focusController().focusedOrMainFrame();
if (PluginView* pluginView = focusedPluginViewForFrame(frame)) {
pluginView->handleEditingCommand(commandName, argument);
@@ -5170,7 +5172,9 @@
void WebPage::insertTextAsync(const String& text, const EditingRange& replacementEditingRange, InsertTextOptions&& options)
{
- Frame& frame = m_page->focusController().focusedOrMainFrame();
+ platformWillPerformEditingCommand();
+
+ auto& frame = m_page->focusController().focusedOrMainFrame();
Ref<Frame> protector(frame);
@@ -5244,7 +5248,9 @@
void WebPage::setCompositionAsync(const String& text, const Vector<CompositionUnderline>& underlines, const EditingRange& selection, const EditingRange& replacementEditingRange)
{
- Frame& frame = m_page->focusController().focusedOrMainFrame();
+ platformWillPerformEditingCommand();
+
+ auto& frame = m_page->focusController().focusedOrMainFrame();
if (frame.selection().selection().isContentEditable()) {
RefPtr<Range> replacementRange;
@@ -5260,6 +5266,8 @@
void WebPage::confirmCompositionAsync()
{
+ platformWillPerformEditingCommand();
+
Frame& frame = m_page->focusController().focusedOrMainFrame();
frame.editor().confirmComposition();
}
diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
index 3ee3a89..164f91c 100644
--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
@@ -1225,6 +1225,7 @@
void platformReinitialize();
void platformDetach();
void platformEditorState(WebCore::Frame&, EditorState& result, IncludePostLayoutDataHint) const;
+ void platformWillPerformEditingCommand();
void sendEditorStateUpdate();
#if PLATFORM(COCOA)
@@ -1957,5 +1958,9 @@
#endif
};
+#if !PLATFORM(IOS_FAMILY)
+inline void WebPage::platformWillPerformEditingCommand() { }
+#endif
+
} // namespace WebKit
diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
index 7e2ada6..cf4f764 100644
--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
@@ -63,6 +63,7 @@
#import <WebCore/AutofillElements.h>
#import <WebCore/Chrome.h>
#import <WebCore/ContentChangeObserver.h>
+#import <WebCore/DOMTimerHoldingTank.h>
#import <WebCore/DataDetection.h>
#import <WebCore/DiagnosticLoggingClient.h>
#import <WebCore/DiagnosticLoggingKeys.h>
@@ -275,6 +276,15 @@
}
}
+void WebPage::platformWillPerformEditingCommand()
+{
+ auto& frame = m_page->focusController().focusedOrMainFrame();
+ if (auto* document = frame.document()) {
+ if (auto* holdingTank = document->domTimerHoldingTankIfExists())
+ holdingTank->removeAll();
+ }
+}
+
FloatSize WebPage::screenSize() const
{
return m_screenSize;
