Summary: | Do not handle open attribute removal the same as HTMLDialogElement::close() | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Nguyen (:ntim) <ntim> | ||||||||||||||
Component: | DOM | Assignee: | Tim Nguyen (:ntim) <ntim> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | cdumez, changseok, esprehn+autocc, ews-watchlist, gyuyoung.kim, webkit-bug-importer | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 227986 | ||||||||||||||||
Bug Blocks: | 84635 | ||||||||||||||||
Attachments: |
|
Description
Tim Nguyen (:ntim)
2021-07-12 05:30:47 PDT
Created attachment 433412 [details]
Patch
Comment on attachment 433412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433412&action=review > Source/WebCore/ChangeLog:3 > + <dialog> element: do not perform close() method steps when removing open attribute. Missing tests. Comment on attachment 433412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433412&action=review r- due to lack of testing. > Source/WebCore/html/HTMLDialogElement.cpp:64 > + // TODO(webkit.org/b/227537): Add steps 3 & 4 No TODO comments in WebKit > Source/WebCore/html/HTMLDialogElement.cpp:81 > + // FIXME(webkit.org/b/227907) Bad format, please see https://webkit.org/code-style-guidelines/#comments. > Source/WebCore/html/HTMLDialogElement.cpp:84 > + // TODO(webkit.org/b/227537): Add steps 8 & 9 No TODO comments in WebKit: https://webkit.org/code-style-guidelines/#comments > Source/WebCore/html/HTMLDialogElement.cpp:89 > +void HTMLDialogElement::close(const String& result) Please explain in the change log the changes to this function as they seem non-trivial. > Source/WebCore/html/HTMLDialogElement.cpp:101 > + // FIXME(webkit.org/b/227907) This is not how we write FIXME comments: https://webkit.org/code-style-guidelines/#comments. Please write a proper sentence ending with a period. > Source/WebCore/html/HTMLDialogElement.cpp:104 > + // TODO(webkit.org/b/227537): Add step 6 No TODO comments in WebKit: https://webkit.org/code-style-guidelines/#comments. > Source/WebCore/html/HTMLDialogElement.h:43 > + const String& returnValue() { return m_returnValue; } should be marked const. Created attachment 433432 [details]
Patch
Created attachment 433433 [details]
Patch
Created attachment 433438 [details]
Patch
Created attachment 433439 [details]
Patch
Created attachment 433580 [details]
Patch
Committed r279950 (239693@main): <https://commits.webkit.org/239693@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433580 [details]. |