WebKit should not copy session storage when a new window opens with rel="noopener". https://html.spec.whatwg.org/multipage/browsers.html#copy-session-storage Firefox and the Web Platform tests (wpt/webstorage/storage_session_window_noopener) get this right; Safari and Chrome (https://crbug.com/771959) get this wrong. I'd like to fix this for Chrome, and getting it fixed in Safari will help. Demo/Repro: 1. Visit https://webdbg.com/test/sessions/ 2. Pick a color in the dropdown. Observe SessionStorage value shown in page. 3. Click 'open this page in ... noopener' link OBSERVE: The new page shows the sessionStorage value was copied from the original window.
<rdar://problem/71286606>
Thanks for filing, Eric! Cc’ing Sihui.
This change has landed in Chromium 89, slated for stable release around March 2021.
(In reply to Eric Lawrence (MSFT) from comment #3) > This change has landed in Chromium 89, slated for stable release around > March 2021. Thanks for the info. I have it implemented, just working on tests now.
Created attachment 415047 [details] Patch
Comment on attachment 415047 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415047&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/SessionStorage.mm:54 > +- (void)webView:(WKWebView *)webView runJavaScriptConfirmPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(BOOL result))completionHandler TestUIDelegate has waitForAlert. We could so something similar instead of making another global done boolean that prevents us from unifying sources. > Tools/TestWebKitAPI/Tests/WebKitCocoa/SessionStorage.mm:67 > + sharedUIDelegate = adoptNS([[SessionStorageUIDelegate alloc] init]); This could just be a function-scoped static variable.
Comment on attachment 415047 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415047&action=review >> Tools/TestWebKitAPI/Tests/WebKitCocoa/SessionStorage.mm:54 >> +- (void)webView:(WKWebView *)webView runJavaScriptConfirmPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(BOOL result))completionHandler > > TestUIDelegate has waitForAlert. We could so something similar instead of making another global done boolean that prevents us from unifying sources. I wasn't familiar this TestUIDelegate's waitForAlert. Seems trivial to use it for the main WebView. However, how would I use it for the new webview that gets opened? >> Tools/TestWebKitAPI/Tests/WebKitCocoa/SessionStorage.mm:67 >> + sharedUIDelegate = adoptNS([[SessionStorageUIDelegate alloc] init]); > > This could just be a function-scoped static variable. I don't think so? It is used in createWebViewWithConfiguration above to set the delegate on the new webView. We use the same UIDelegate for the new WebView.
(In reply to Chris Dumez from comment #7) > Comment on attachment 415047 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=415047&action=review > > >> Tools/TestWebKitAPI/Tests/WebKitCocoa/SessionStorage.mm:54 > >> +- (void)webView:(WKWebView *)webView runJavaScriptConfirmPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(BOOL result))completionHandler > > > > TestUIDelegate has waitForAlert. We could so something similar instead of making another global done boolean that prevents us from unifying sources. > > I wasn't familiar this TestUIDelegate's waitForAlert. Seems trivial to use > it for the main WebView. However, how would I use it for the new webview > that gets opened? > > >> Tools/TestWebKitAPI/Tests/WebKitCocoa/SessionStorage.mm:67 > >> + sharedUIDelegate = adoptNS([[SessionStorageUIDelegate alloc] init]); > > > > This could just be a function-scoped static variable. > > I don't think so? It is used in createWebViewWithConfiguration above to set > the delegate on the new webView. We use the same UIDelegate for the new > WebView. Also TestUIDelegate does not seem to implement createWebViewWithConfiguration.
It also doesn't implement runJavaScriptConfirmPanelWithMessage but it could.
(In reply to Alex Christensen from comment #9) > It also doesn't implement runJavaScriptConfirmPanelWithMessage but it could. It could, but how about the other issues I mentioned? How do I use [TestUIDelegate waitForAlert:] for the newly opened window? Seems like it would be racy. I can wait for openedWebView to become non-null. But then calling [openedWebView.UIDelegate waitForAlert] seems like it would be racy because it is possible the alert was already shown by this point.
These are just nits about the test variable scopes. Feel free to commit as-is. sharedUIDelegate would need its own function to be function scoped and shared between two other functions. The main test may need to wait for createWebViewWithConfiguration to be called then wait for a message after that on the UIDelegate that is set. Could be done.
Created attachment 415056 [details] Patch
(In reply to Alex Christensen from comment #11) > These are just nits about the test variable scopes. Feel free to commit > as-is. > > sharedUIDelegate would need its own function to be function scoped and > shared between two other functions. > > The main test may need to wait for createWebViewWithConfiguration to be > called then wait for a message after that on the UIDelegate that is set. > Could be done. I think this latest patch iteration addresses your comments. Let me know if you think it can still be improved.
Committed r270273: <https://trac.webkit.org/changeset/270273> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415056 [details].