Summary: | document.readyState should be "complete" after calling DOMParser.parseFromString() | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||
Component: | DOM | Assignee: | Chris Dumez <cdumez> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, commit-queue, darin, esprehn+autocc, ews-watchlist, ggaren, kangil.han, koivisto, rniwa, sam, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | 227850 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Chris Dumez
2021-07-09 16:12:05 PDT
Created attachment 433246 [details]
Patch
Created attachment 433248 [details]
Patch
Comment on attachment 433248 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433248&action=review > Source/WebCore/dom/Document.cpp:3106 > + setReadyState(Complete); Does this also impact document.close? Comment on attachment 433248 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433248&action=review > Source/WebCore/ChangeLog:26 > + * dom/Document.cpp: > + (WebCore::Document::explicitClose): > + explicitClose() normally calls checkCompleted() which calls FrameLoader::checkCompleted(), > + which ends up setting the document's ready state to "complete" and then calling > + Document::implicitClose(). However, when the document has no frame (which is the case > + for a document just created via DOMParser.parseFromString()), we would call > + Document::implicitClose() directly, since we don't have a FrameLoader. As a result, > + the document's ready state would stay "interactive". To address the issue, we now set > + the document's ready state to "complete" before calling implicitClose(), similarly to > + what FrameLoader::checkCompleted() would have done. Seems good. I wish we had a better model of dealing with frameless documents, so we aren't duplicating functionality. (In reply to Ryosuke Niwa from comment #3) > Comment on attachment 433248 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=433248&action=review > > > Source/WebCore/dom/Document.cpp:3106 > > + setReadyState(Complete); > > Does this also impact document.close? Not super familiar with `document.close()` but looking at the code, it seems to only have an impact on documents created by JS because of this early return: ``` if (!scriptableDocumentParser() || !scriptableDocumentParser()->wasCreatedByScript() || !scriptableDocumentParser()->isParsing()) return; ``` Documents created by scripts should now have a readyState that is "complete" after my patch so calling close() on the document later on should not make a difference on the readyState since it is already "complete". Am I missing something? (In reply to Chris Dumez from comment #5) > (In reply to Ryosuke Niwa from comment #3) > > Comment on attachment 433248 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=433248&action=review > > > > > Source/WebCore/dom/Document.cpp:3106 > > > + setReadyState(Complete); > > > > Does this also impact document.close? > > Not super familiar with `document.close()` but looking at the code, it seems > to only have an impact on documents created by JS because of this early > return: > ``` > if (!scriptableDocumentParser() || > !scriptableDocumentParser()->wasCreatedByScript() || > !scriptableDocumentParser()->isParsing()) > return; > ``` > > Documents created by scripts should now have a readyState that is "complete" > after my patch so calling close() on the document later on should not make a > difference on the readyState since it is already "complete". Am I missing > something? How about document.implementation.createDocument/createHTMLDocument? (In reply to Ryosuke Niwa from comment #6) > (In reply to Chris Dumez from comment #5) > > (In reply to Ryosuke Niwa from comment #3) > > > Comment on attachment 433248 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=433248&action=review > > > > > > > Source/WebCore/dom/Document.cpp:3106 > > > > + setReadyState(Complete); > > > > > > Does this also impact document.close? > > > > Not super familiar with `document.close()` but looking at the code, it seems > > to only have an impact on documents created by JS because of this early > > return: > > ``` > > if (!scriptableDocumentParser() || > > !scriptableDocumentParser()->wasCreatedByScript() || > > !scriptableDocumentParser()->isParsing()) > > return; > > ``` > > > > Documents created by scripts should now have a readyState that is "complete" > > after my patch so calling close() on the document later on should not make a > > difference on the readyState since it is already "complete". Am I missing > > something? > > How about document.implementation.createDocument/createHTMLDocument? I think I'll add some test and make sure our behavior is consistent with other engines. Created attachment 433254 [details]
Patch
Committed r279803 (239567@main): <https://commits.webkit.org/239567@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433254 [details]. Re-opened since this is blocked by bug 227850 Created attachment 433272 [details]
Patch
Committed r279814 (239575@main): <https://commits.webkit.org/239575@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433272 [details]. |