1172928 - Window.close() does not close the tab

Status: RESOLVED FIXED

(Firefox for iOS :: General, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

This is a followup bug for 1124942, which has not landed yet.

STR:

* Go to https://mozillians.org
* Login with persona
* The Persona login screen should appear in a new tab
* Finish the login

Expected:

The persona login tab closes and you go back to the tab you came from.

Actual:

Persona just sits there. But if you manually close the tab and you go back to the mozillians.org tab, you are actually correctly logged in.


Not sure what piece we miss to support window.close(), may be some WKWebView delegate that we need to implement?

Comment 1

[James Hugman [:jhugman] [@jhugman]]

It looks like that there is no public API for detecting window.close events in WKWebview frameworks.

There is some code in Webkit https://bugs.webkit.org/show_bug.cgi?id=137008 demonstrating a private API, but this doesn't seem to be working for us in Swift. Plus: private APIs.

Comment 2

[James Hugman [:jhugman] [@jhugman]]

Attachment: Work in progress. Catching window.close events in user scripts (content js).

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I filed https://bugs.webkit.org/show_bug.cgi?id=146638

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

The bug that I filed yesterday was closed as a dupe of https://bugs.webkit.org/show_bug.cgi?id=145957 which adds a webViewDidClose: delegate method to the WKUIDelegate. So that will allow us to do this without any JavaScript hacks.

What I would like to suggest is that we do a simple solution now, like patching window.close(), which will probably catch 75% of the use-cases. And then replace that JavaScript solution with the new WKUIDelegate when we are ready for iOS9 later this year.

Comment 5

[Richard Newman [:rnewman]]

One-line fix in iOS 9. Let's punt this to 2.0.

Comment 6

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/758

Work in progress but please take a look anyway. This may catch most cases. Except persona as noted below.


This patches window.close to send us a callback so that the native code can close the tab. This work for the general case. It does not work for Persona because Persona is insanely complicated and they probably do something special.

Comment 7

[Brian Nicholson (:bnicholson)]

Comment on attachment 8634771 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/758

LGTM with a few comments.

Comment 8

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Couple of notes:

This works correctly on for example http://www.permadi.com/tutorial/facebook-js-oauth-popup-centered/ which opens a new tab with a Facebook login. After logging in the tab closes correctly and you are back at the original tab and logged in.

On Persona this fails, but now I think this may not be an error in this code but an issue with Persona. I see a message about origin login.persona.org not matching login.mozilla.org. (This appears in the console for login.persona.org - dont forget to attach the debugger to the right tab)

Comment 9

[Brian Nicholson (:bnicholson)]

Assuming we want to land this since it handles some cases on iOS 8. Filed bug 1186030 for iOS 9 specifically.

Comment 10

[Richard Newman [:rnewman]]

Stefan, this needs landing.

Comment 11

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I'll deal with the review comments on tuesday and land it then.

Is there someone on the (old) Persona team who we can loop in to see if we can find out why this patch does not work for the Persona popup? Persona does crazy things, but I don't know enough about JS to understand what specifically.

Comment 12

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I have addressed the review comments and landed this. Couple of notes:

 * There is a test page at http://people.mozilla.org/~sarentz/t/wo.html that shows this fix in action.
 * For testing it is important to know that WebKit only lets you window.close() webviews that have a parent/child relationship. You can't just window.close() any window.
 * This does not work with Persona yet because I assume Persona does some more complicated things. I have reached out to people to get that fixed too, which can be done as a followup bug.

https://github.com/mozilla/firefox-ios/commit/43e50b6c697196c260a0fd1472da3210b98592b2

Comment 13

[Dan Callahan [:callahad]]

Shane Tomlinson would be your best bet from the legacy Persona team. You might want to approach Ryan Kelly or Chris Karlof about getting some of Shane's time to look into this.

Comment 14

[Bhavna]

Look like this issue is produce again in latest build.
(BUG RISES)

Please have a look into this issue for more detail.
https://github.com/mozilla-mobile/firefox-ios/issues/2481

I also did pull request for solution
https://github.com/mozilla-mobile/firefox-ios/pull/2484

Here is a test url i created to test.
https://bhavnapanchal.github.io/firefoxpopuptest.github.io/

Comment 15

[Aaron Train [:aaronmt]]

Please file a new bug.