After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759044 - Portal helper doesn't redirect on login button click
Portal helper doesn't redirect on login button click
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: portal-helper
3.18.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2015-12-04 19:29 UTC by Ties Jan Hefting
Modified: 2017-04-06 10:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
captive-portal-html (7.49 KB, text/plain)
2015-12-04 19:29 UTC, Ties Jan Hefting
  Details
captive-portal-bundle (30.84 KB, message/rfc822)
2017-01-19 08:50 UTC, Ties Jan Hefting
  Details
v1 Load new window if navigation is user initiated (1.42 KB, patch)
2017-03-27 04:55 UTC, Catalin Iacob
committed Details | Review

Description Ties Jan Hefting 2015-12-04 19:29:09 UTC
Created attachment 316788 [details]
captive-portal-html

Situation:
The captive portal of a Dutch railway service is detected perfectly
by NetworkManager, and GNOME Shell shows the portal-helper to login.
When I click the login button, nothing happens.

Expected:
Show the next page that says I am connected to the WiFi network.

Possible underlying problem:
If I browse in Firefox to the captive portal manually the button
works. One thing I noticed is that Firefox opens a new tab after
I've logged in successfully. In the source code, this button submits
a form using JavaScript. The form only consists of the checkbox. In
the <form>-tag the attribute 'target' is set to 'connectiontarget',
but I can't find that elsewhere in the source code, nor do I know
what the HTML spec says about this non-standard value.

Extra:
- I use Fedora 23 (64-bit) with GNOME Shell 3.18.3 on a Lenovo
  Thinkpad T550 laptop
- Screenshots can be found here: http://imgur.com/a/BfmY
- The source code of the login page is attached
Comment 1 Ties Jan Hefting 2015-12-04 19:30:56 UTC
Sorry. Error in the link to screenshots.
New link: http://imgur.com/a/BfmYF
Comment 2 Bastien Nocera 2017-01-18 16:32:35 UTC
I'm not sure whether you'd be able to reproduce the problem now, but it would be nice to have at least all the source code that's eventually being run, to be able to reproduce the problem.

It would be great if you could see whether the problem still occurs, because:
- the web page could have changed
- webkitgtk has changed

If you manage to reproduce the problem, please test it with MiniBrowser (it's shipped in the webkitgtk4-devel package in Fedora) so we can see whether the problem is specific to the portal helper, and attach an archive of the page (epiphany will save the page and all the ancillary files in one bundle).
Comment 3 Ties Jan Hefting 2017-01-19 08:50:15 UTC
Created attachment 343783 [details]
captive-portal-bundle

Thank you for your reply. I am still able to reproduce the problem using my Fedora 25, GNOME 3.22.2 system.

I tried to connect through the MiniBrowser as you described. When I check the tickbox and click the blue-yellow connect button, it opens a new window in which it tells me I'm connected. You can find a screenshot here: http://imgur.com/a/DaDBF

Attached you'll find the bundle created by Epiphany. I hope I saved it correctly. Please let me know if you need more info, I'd be happy to help.
Comment 4 Catalin Iacob 2017-03-27 04:55:32 UTC
Created attachment 348767 [details] [review]
v1 Load new window if navigation is user initiated

I'm a commuter on Dutch trains, saw this in Fedora 25 and recently spent quite some time diagnosing it (and learning how to build gnome-shell, how to test my changes and so on).

They use some horribly complicated Apache Wicket based thing server side which does crazy Javascript and DOM modifications, I at least can't trace at all what the web page is actually doing. They indeed require the new window they pop up after you click the "connect" button to be loaded in order to let you through. After you click you get back a "you're now connected message", browsers open that in a new tab.

I came up with the attached patch against master which does work for me.

Something to keep in mind is that the patch uses the navigation action stuff that appeared only in webkitgtk 2.6 (released September 2014). Couldn't find a clear minimum required webkitgtk version. With the dynamic gobject introspection one gets late runtime errors rather than build errors so it might bite some users.

Finally, theoretically this could break some other portals which open some other window unrelated to the login process (for example for ads or whatever) when clicking something. But phones also only have one window for the portal UI so I imagine they would test on phones and not do something that breaks those.
Comment 5 Catalin Iacob 2017-03-27 16:45:06 UTC
(In reply to Catalin Iacob from comment #4)
> Something to keep in mind is that the patch uses the navigation action stuff
> that appeared only in webkitgtk 2.6 (released September 2014).

Over in bug780453 there's talk of requiring webkitgtk 2.16 released a few days ago so I gather that requiring 2.6 from 2014 is fine :)
Comment 6 Bastien Nocera 2017-03-28 12:23:07 UTC
FWIW, patch looks fine to me.
Comment 7 Ties Jan Hefting 2017-03-29 11:37:23 UTC
Thanks for taking the time to look into this! Unfortunately, I don't have had the time yet to learn building gnome-shell, so I cannot test your patch. It does look fine to me.

I can confirm their login page works on my Android phone. Not sure how Android does this: whether it ignores the target attribute, or they do it as you have written in your patch, or a new window replaces the other... However, the fact is: using your patch the target is overridden and this is the only behavioral change, essentially. Therefore I assume it won't get other portals into trouble. :)
Comment 8 Catalin Iacob 2017-03-30 04:49:36 UTC
(In reply to Ties Jan Hefting from comment #7)
> However, the fact is: using your patch the target is overridden and this is
> the only behavioral change, essentially. Therefore I assume it won't get
> other portals into trouble. :)

It could get into trouble if the page is written such that when clicking something a popup unrelated to the login process would open. Before, this would have been ignored, with this patch, the popup would replace the page and you would have no possibility to go back and finish the authentication.

But I do agree that such a concern is mostly theoretical, pages have stopped doing that a long time ago and portal pages must work with the mobile portal helpers which also have a single window unlike a real browser.
Comment 9 Catalin Iacob 2017-04-05 15:30:41 UTC
(In reply to Bastien Nocera from comment #6)
> FWIW, patch looks fine to me.

So Bastien, will you push it then? Otherwise what's the process?
Comment 10 Bastien Nocera 2017-04-05 15:52:31 UTC
(In reply to Catalin Iacob from comment #9)
> (In reply to Bastien Nocera from comment #6)
> > FWIW, patch looks fine to me.
> 
> So Bastien, will you push it then? Otherwise what's the process?

It needs to be reviewed by a gnome-shell developer. I'm not one despite what the label next to my name might say (I have triage rights on the component, that's all).
Comment 11 Florian Müllner 2017-04-05 16:27:37 UTC
Review of attachment 348767 [details] [review]:

LGTM