From e06cac856583de763def0808df48f68c71b96f5e Mon Sep 17 00:00:00 2001
From: Benjamin Franzke <ben@bnf.dev>
Date: Tue, 18 Jun 2024 05:59:35 +0200
Subject: [PATCH] [BUGFIX] Avoid race condition in DocumentService.ready()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Improve document-service responsiveness by relying on
`DOMContentLoaded` and `document.readyState` >= `interactive`.
1) Handle non-loading state as "ready" to avoid waiting for `complete`.
`document.readyState` has three states:
* `loading`
The document is still loading.
* `interactive`
The document has finished loading and the document
has been parsed but sub-resources such as scripts, images,
stylesheets and frames are still loading. The state indicates
that the DOMContentLoaded event is about to fire.
* `complete`
The document and all sub-resources have finished loading.
The state indicates that the load event is about to fire.
If DocumentService.ready was called in "interactive" state we have
been skipping this state as we only considered `complete` to be
the "ready" state in this case.
This is wrong as we actually wait for the DOMContentLoaded if we
are launched in the initial readyState (`loading`), that means
an initial `interactive` must be understood as:
`DOMContentLoaded` has already been fired as state is `interactive`
We should actually avoid waiting for `complete` entirely, as document
readyState `interactive` means "ready" in terms of document parsing.
We're not interested in images or async scripts that are still
loading and want event listeners to be registered as early as
possible.
With this improvement applied we can also drop the (now) unneeded
`load` event-listener, as we are no longer skipping interactive
state (this made the `load`-listener necessary previously).
Note that the `load` workaround was previously needed for IE<=10 as
those versions set `document.readyState` to `interactive` prior to
the `DOMContentLoaded` event. This applied workaround becomes a race
condition once `load` never happens and that is the case with a
recent google chrome regression which causes iframe documents to
never "complete": https://issues.chromium.org/issues/347724924
2) Drop the timeout logic. It is no longer needed now that we only need
to wait for `DOMContentLoaded` – which will always fire (or already
fired) – instead of the `load` event.
3) Avoid creation of new promises for every invocation of the
ready-method. A promise can be reused without side effects by
multiple consumers. This avoids creating a lot of event listeners
(and previously timers).
4) Remove unneeded document and window references.
Releases: main, 12.4, 11.5
Resolves: #104135
Related: #104139
Change-Id: I42c86961405f8a5c346c17ea429a288a85d58f8a
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84742
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Andreas Kienast <a.fernandez@scripting-base.de>
Tested-by: Benjamin Franzke <ben@bnf.dev>
Reviewed-by: Andreas Kienast <a.fernandez@scripting-base.de>
Reviewed-by: Benjamin Franzke <ben@bnf.dev>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Jasmina Ließmann <minapokhalo+typo3@gmail.com>
Tested-by: Oliver Bartsch <bo@cedev.de>
---
.../TypeScript/core/document-service.ts | 42 ++++---------------
.../Public/JavaScript/document-service.js | 2 +-
2 files changed, 10 insertions(+), 34 deletions(-)
diff --git a/Build/Sources/TypeScript/core/document-service.ts b/Build/Sources/TypeScript/core/document-service.ts
index 7f07b17623eb..3d346a212d26 100644
--- a/Build/Sources/TypeScript/core/document-service.ts
+++ b/Build/Sources/TypeScript/core/document-service.ts
@@ -16,42 +16,18 @@
* @exports @typo3/core/document-service
*/
class DocumentService {
- private readonly windowRef: Window;
- private readonly documentRef: Document;
+ private promise: Promise<Document> = null;
- /**
- * @param {Window} windowRef
- * @param {Document} documentRef
- */
- constructor(windowRef: Window = window, documentRef: Document = document) {
- this.windowRef = windowRef;
- this.documentRef = documentRef;
+ public ready(): Promise<Document> {
+ return this.promise ?? (this.promise = this.createPromise());
}
- ready(): Promise<Document> {
- return new Promise<Document>((resolve: (document: Document) => void, reject: (document: Document) => void) => {
- if (this.documentRef.readyState === 'complete') {
- resolve(this.documentRef);
- } else {
- // timeout & reject after 30 seconds
- const timer = setTimeout((): void => {
- clearListeners();
- reject(this.documentRef);
- }, 30000);
- const clearListeners = (): void => {
- this.windowRef.removeEventListener('load', delegate);
- this.documentRef.removeEventListener('DOMContentLoaded', delegate);
- };
- const delegate = (): void => {
- clearListeners();
- clearTimeout(timer);
- resolve(this.documentRef);
- };
- this.windowRef.addEventListener('load', delegate);
- this.documentRef.addEventListener('DOMContentLoaded', delegate);
- }
-
- });
+ private async createPromise(): Promise<Document> {
+ if (document.readyState !== 'loading') {
+ return document;
+ }
+ await new Promise<void>(resolve => document.addEventListener('DOMContentLoaded', () => resolve(), { once: true }));
+ return document;
}
}
diff --git a/typo3/sysext/core/Resources/Public/JavaScript/document-service.js b/typo3/sysext/core/Resources/Public/JavaScript/document-service.js
index 7264f00c773c..743ead5d53b4 100644
--- a/typo3/sysext/core/Resources/Public/JavaScript/document-service.js
+++ b/typo3/sysext/core/Resources/Public/JavaScript/document-service.js
@@ -10,4 +10,4 @@
*
* The TYPO3 project - inspiring people to share!
*/
-class DocumentService{constructor(e=window,t=document){this.windowRef=e,this.documentRef=t}ready(){return new Promise(((e,t)=>{if("complete"===this.documentRef.readyState)e(this.documentRef);else{const n=setTimeout((()=>{o(),t(this.documentRef)}),3e4),o=()=>{this.windowRef.removeEventListener("load",d),this.documentRef.removeEventListener("DOMContentLoaded",d)},d=()=>{o(),clearTimeout(n),e(this.documentRef)};this.windowRef.addEventListener("load",d),this.documentRef.addEventListener("DOMContentLoaded",d)}}))}}const documentService=new DocumentService;export default documentService;
\ No newline at end of file
+class DocumentService{constructor(){this.promise=null}ready(){return this.promise??(this.promise=this.createPromise())}async createPromise(){return"loading"!==document.readyState||await new Promise((e=>document.addEventListener("DOMContentLoaded",(()=>e()),{once:!0}))),document}}const documentService=new DocumentService;export default documentService;
\ No newline at end of file
--
GitLab