On Sun, 27 Feb 2022 05:43:27 GMT, Jay Bhaskar <d...@openjdk.java.net> wrote:
>> Issue: The current implementation of DOMWindow ::localStorage(..) returns >> null pointer in case of page is being closed. >> Fix: It should not return nullptr , as per the [w3c web storage >> spec](https://www.w3.org/TR/2016/REC-webstorage-20160419/) >> "User agents should expire data from the local storage areas only for >> security reasons or when requested to do so by the user. User agents should >> always avoid deleting data while a script that could access that data is >> running." > > Jay Bhaskar has updated the pull request incrementally with one additional > commit since the last revision: > > Add Review Changes The fix and test look good now. I left a couple cleanup comments. I'll reapprove once you fix them. One other minor suggestion: Since all of the test methods now have the same three init statements at the beginning, you might want to make `webEngine` an instance field (in which case you wouldn't need to pass it to `checkLocalStorageAfterWindowClose`), and do the initialization in a `@Before` method. It's up to you. modules/javafx.web/src/main/native/Source/WebCore/page/DOMWindow.cpp line 851: > 849: > 850: // FIXME: We should consider supporting access/modification to local > storage > 851: // after calling window.close(). See > <https://bugs.webkit.org/show_bug.cgi?id=135330>. The change of moving this block back here looks good. You can delete the obsolete comment now, right? modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java line 142: > 140: }); > 141: } > 142: } Minor: can you restore the newline at the end of this file? ------------- Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/703
