Bug 527738 (webkit2portPostData) - [Webkit2] implement setUrl(...POST..) support. (snippet 330)
Summary: [Webkit2] implement setUrl(...POST..) support. (snippet 330)
Status: RESOLVED FIXED
Alias: webkit2portPostData
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.8   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.8 M7   Edit
Assignee: Roland Grunberg CLA
QA Contact: Leo Ufimtsev CLA
URL:
Whiteboard: RHT
Keywords:
Depends on:
Blocks: webkit2Port4.8
  Show dependency tree
 
Reported: 2017-11-24 15:06 EST by Leo Ufimtsev CLA
Modified: 2018-03-27 14:48 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Leo Ufimtsev CLA 2017-11-24 15:06:23 EST
Snippet 330 has 'set headers' and 'post data' button.

Sending data over 'postData' isn't working on webkit2.
- On webkit1, bugzilla can be queries with postData
- On webkit2, bugzilla opens with an error message about missing post data.
Upon code inspection, it seems that postData has not been ported to webkit2.
(but headers button seems to work.)
Comment 1 Leo Ufimtsev CLA 2017-11-24 15:07:36 EST
i.e, setUrl(... postData ... ). (but would be good to check headers also).
Comment 2 Roland Grunberg CLA 2017-11-29 11:09:53 EST
One slight blocker for this is the lack of API for accessing the HTTP body itself in WebKitURIRequest (https://webkitgtk.org/reference/webkit2gtk/stable/WebKitURIRequest.html). We can access the HTTP Headers (as SoupMessageHeaders), and even the HTTP method (which we can change from GET to POST), but the POST data must be added to the body of the message and we don't have access to that.

In WebKit1, there was a method that gave us the SoupMessage, which contained the method, headers, and body of the request. I'll see if there's some workaround.
Comment 3 Roland Grunberg CLA 2017-11-29 11:36:35 EST
Looks like Sami discovered the same issue, and made pretty much the same request at https://bugs.webkit.org/show_bug.cgi?id=129379 . I don't see any progress though.
Comment 4 Leo Ufimtsev CLA 2017-11-29 17:10:09 EST
From what I gather in the following discussion:
http://webkit-gtk.webkit.narkive.com/kppfqah2/networkrequest-and-networkresponse-in-webkit2-api

It was decided not to expose SoupMessage, only some of it's properties.
Comment 5 Leo Ufimtsev CLA 2017-11-29 17:51:03 EST
One possible short term hack-around while we wait for webkitgtk folks to implement api is to put POST-data into URL (i.e GET).

E.g:
> .....
> @Override
> public boolean setUrl (String url, String postData, String[] headers) {
>     this.postData = postData;
>     this.headers = headers;
>     if (WEBKIT2) { 
>          if (postData != null) {
>              // Print some warning about unfinished api here.
>              url = url + "?" + postData;    // append 
>          }
>     }
>   ...

With this Snippet330 works. 
It'll work so long as postData is less than 2,048 characters long. (where as post can be up to 2GB).

This is far from ideal, but might be better than nothing, especially considering SWT Webrowser is only used for smaller/simple apps, it might cover a lot of use cases.
Comment 6 Roland Grunberg CLA 2017-11-30 10:57:37 EST
(In reply to Leo Ufimtsev from comment #5)
> One possible short term hack-around while we wait for webkitgtk folks to
> implement api is to put POST-data into URL (i.e GET).
> 
> E.g:
> > .....
> > @Override
> > public boolean setUrl (String url, String postData, String[] headers) {
> >     this.postData = postData;
> >     this.headers = headers;
> >     if (WEBKIT2) { 
> >          if (postData != null) {
> >              // Print some warning about unfinished api here.
> >              url = url + "?" + postData;    // append 
> >          }
> >     }
> >   ...
> 
> With this Snippet330 works. 
> It'll work so long as postData is less than 2,048 characters long. (where as
> post can be up to 2GB).
> 
> This is far from ideal, but might be better than nothing, especially
> considering SWT Webrowser is only used for smaller/simple apps, it might
> cover a lot of use cases.

This definitely seems like it's better than nothing but I wonder by how much. I believe bugzilla supports sending a query string over a GET request but not all services will accept data as both POST or GET, and in the case of file uploads, it won't be possible.
Comment 7 Eclipse Genie CLA 2017-12-01 15:10:13 EST
New Gerrit change created: https://git.eclipse.org/r/112732
Comment 8 Leo Ufimtsev CLA 2017-12-12 14:37:40 EST
Implemented via workaround. (use java http request).

We will request api from webkitgtk folks and track enchancement via:
528550 – [Webkit2] Implement setUrl(..postData..) via webkitgtk api instead of java when/if webkitgtk api becomes available. 
https://bugs.eclipse.org/bugs/show_bug.cgi?id=528550
Comment 9 Leo Ufimtsev CLA 2017-12-12 14:38:36 EST
@Roland. Kudos for creative workaround.
Comment 11 Eclipse Genie CLA 2017-12-15 14:18:04 EST
New Gerrit change created: https://git.eclipse.org/r/113501
Comment 13 Lars Vogel CLA 2018-03-27 05:25:12 EDT
The added unit test org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser
test_setUrl_remote_with_post fails for me on Windows 10 if I ran the AllTests test suite. 

org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser
test_setUrl_remote_with_post(org.eclipse.swt.tests.junit.Test_org_eclipse_swt_browser_Browser)
java.lang.AssertionError: Response data contained 247 chars.

@Roland, AFAICS the test is from you. Can you please have a look?
Comment 14 Nikita Nemkin CLA 2018-03-27 05:55:29 EDT
This test grabs page contents without waiting for it to load completely. (Instead it waits for the page title to change).

For local pages this isn't noticeable, but test_setUrl_remote_with_post hits a slow loading remote page (bugs.eclipse.org).
Comment 15 Leo Ufimtsev CLA 2018-03-27 09:39:15 EDT
Thanks for noticing this. We'll take a look.
Comment 16 Roland Grunberg CLA 2018-03-27 11:09:14 EDT
(In reply to Nikita Nemkin from comment #14)
> This test grabs page contents without waiting for it to load completely.
> (Instead it waits for the page title to change).
> 
> For local pages this isn't noticeable, but test_setUrl_remote_with_post hits
> a slow loading remote page (bugs.eclipse.org).

Yes, this sounds familiar. I remember having a lot of difficulty finding some kind of content/request listener which actually would get activated when the page load was complete, so ultimately I had to use a status listener.

Going from my memory it may have been specific to windows would have to look again to see.
Comment 17 Nikita Nemkin CLA 2018-03-27 11:15:50 EDT
There is Browser.addProgressListener() with ProgressListener.completed() callback. Looks exactly like what's needed here (assuming it works).
Comment 18 Eclipse Genie CLA 2018-03-27 12:09:14 EDT
New Gerrit change created: https://git.eclipse.org/r/120283
Comment 19 Roland Grunberg CLA 2018-03-27 12:11:29 EDT
(In reply to Nikita Nemkin from comment #17)
> There is Browser.addProgressListener() with ProgressListener.completed()
> callback. Looks exactly like what's needed here (assuming it works).

Yes, I can confirm his will work on Linux. In fact I can see ProgressListener used in other places as well.

Let me just confirm it also works on win32 and macos. Maybe I simply used it incorrectly the first time.
Comment 20 Roland Grunberg CLA 2018-03-27 13:44:34 EDT
(In reply to Roland Grunberg from comment #19)
> (In reply to Nikita Nemkin from comment #17)
> > There is Browser.addProgressListener() with ProgressListener.completed()
> > callback. Looks exactly like what's needed here (assuming it works).
> 
> Yes, I can confirm his will work on Linux. In fact I can see
> ProgressListener used in other places as well.
> 
> Let me just confirm it also works on win32 and macos. Maybe I simply used it
> incorrectly the first time.

Works as well on win32 and macos, so should be fine to review now.
Comment 21 Lars Vogel CLA 2018-03-27 14:48:39 EDT
Thanks, Roland, Nikita and Leo. This fixes the Test_org_eclipse_swt_browser_Browser test for me.