Issue with popup window

[erikos]

Page: ​http://www.dhl.de/en/paket/kundenservice/sendungsverfolgung.html

Description: This page does open a new page to show the search result.

Result: does not open on the XO, does work in Epiphany on the XO.

Test cases for wanted/unwanted popup windows:

​http://dev.laptop.org/~erikos/wanted.html

​http://dev.laptop.org/~erikos/unwanted.html

[dnarvaez]

This works as expected for me in sugar-build, both F17 and F18.

[dnarvaez]

Actually this works fine for me on the XO too, build 13.

[erikos]

Hi Daniel, if you scroll down a bit on the page ​http://www.dhl.de/en/paket/kundenservice/sendungsverfolgung.html there is a field track&trace. If you enter the code '350043001668' there and click 'Search now' nothing happens. And that result would open in a new tab.

[dnarvaez]

Yeah, sorry the testcases works but not the dhl site.

I had a look at the code and I think our popups implementation is wrong. We need to implement

​http://webkitgtk.org/reference/webkitgtk/stable/webkitgtk-webkitwebview.html#WebKitWebView-create-web-view

(verified that's actually called on dhl)

And I suspect we should only be making policy decisions in new-window-policy-decision-requested.

[dnarvaez]

This is useful btw

​http://git.gnome.org/browse/epiphany/tree/src/ephy-window.c

[erikos]

The two examples I added above ​http://dev.laptop.org/~erikos/wanted.html and ​http://dev.laptop.org/~erikos/unwanted.html does work as expected on the XO. The former does open the new window in a new tab if I click on the button. The latter does block the popup window.

Now, the question is, why the packet example (dhl.de) does work in Epiphany but not in Browse.

I checked as well that the popup page itself can be rendered fine in Browse. If you put in the tracking id in the page ​http://nolp.dhl.de/nextt-online-public/set_identcodes.do you get to the page fine and to the information. So the issue must really be the popup window part.

[erikos]

Thanks for the pointers Daniel, so we are know now where we have to adjust. Let's move this into 0.98.

[humitos]

Replying to erikos:

The two examples I added above ​http://dev.laptop.org/~erikos/wanted.html and ​http://dev.laptop.org/~erikos/unwanted.html does work as expected on the XO. The former does open the new window in a new tab if I click on the button. The latter does block the popup window.

Those examples use Javascript code (window.open(url)) to create a new window with the URL given. I understand that case is already managed by the signal: new-window-policy-decision-requested

• ​http://webkitgtk.org/reference/webkitgtk/stable/webkitgtk-webkitwebview.html#WebKitWebView-new-window-policy-decision-requested

Now, the question is, why the packet example (dhl.de) does work in Epiphany but not in Browse.

The site you mentioned here uses target="_blank" on its form and that is handled by(*): 'create-web-view' signal. That is the signal we are not handling.

I've got a patch that I'm testing to be sure that it cover these cases.

(*) I can't confirm that because I've just create an index.html to test that and it worked on build 13. I don't understand what is the difference between 'new-window-policy-decision-requested' and 'create-web-view'. Another reason could be some strange javascript connected to the submit button that I'm not seeing.

Anyway, I'm uploading the patch that I have here tested on many cases to see if someone else find another case that it doesn't work or to take a look what I did.

[humitos]

Testcase with target="_blank" on a <a> and <form>

[humitos]

Patch that handles 'create-web-view'

[dnarvaez]

Couple of comments.

According to the API docs you should not display the window in create-web-view, you should wait until web-view-ready. It's not clear if that's just to have the features information available, but in doubt I'd follow the docs literally. Web pages do all kinds of weird stuff with popups and it's easy to overlook bugs in testing.

I haven't tested this but I would guess create-web-view will work for all kind of popups. So I think the current new-window-policy-decision-requested code can/should be removed. It doesn't seem right anyway.

[humitos]

Replying to dnarvaez:

According to the API docs you should not display the window in create-web-view, you should wait until web-view-ready. It's not clear if that's just to have the features information available

Yes, I read the docs and this is true. I didn't use that because we don't have "windows" inside Browse and our tabs doesn't have size / position / status and scrollbars properties. So, I think it's no needed.

Do you agree with me?

I haven't tested this but I would guess create-web-view will work for all kind of popups. So I think the current new-window-policy-decision-requested code can/should be removed.

I will try this and I will let you know.

Thanks for your comments.

[dnarvaez]

Replying to humitos:

Replying to dnarvaez:

According to the API docs you should not display the window in create-web-view, you should wait until web-view-ready. It's not clear if that's just to have the features information available

Yes, I read the docs and this is true. I didn't use that because we don't have "windows" inside Browse and our tabs doesn't have size / position / status and scrollbars properties. So, I think it's no needed.

Do you agree with me?

The problem is, I'm not sure without looking at the webkit code that the only reason they suggest to run it later is those features. IMO it would be safer to just show it later... Unless it's really hard to code or something.

[humitos]

v2 - this patch removes the hanlding of the signal 'new-window-policy-decision-requested' and handle 'web-view-ready' to be sure that we can show the WebView

[humitos]

dnarvaez: I've just attached a new version of this patch that removes the 'new-window-policy-decision-requested' and makes use of 'web-view-ready'

[erikos]

Thanks Humitos your patch passes the following cases:

• ​http://dev.laptop.org/~erikos/wanted.html

• ​http://dev.laptop.org/~erikos/unwanted.html

• the dhl packet page

If Daniel agrees with the patch itself, I am ok and we can push it. Looks good to me at first glance.

[dnarvaez]

I have not tested but patch looks good to me in theory!

[manuq]

Good work! Pushed b3d2fa7d .

[greenfeld]

Browse-148 passes these popup testcases in OLPC 13.1.0 os20.

[dnarvaez]

Milestone 0.98 deleted