Bugzilla – Full Text Bug Listing
| Summary: | [Browser][Edge] cookie setting/getting does not work | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Flavio Donze <flavio.donze> |
| Component: | SWT | Assignee: | Nikita Nemkin <nikita> |
| Status: | VERIFIED FIXED | QA Contact: | Niraj Modi <niraj.modi> |
| Severity: | normal | ||
| Priority: | P3 | CC: | digga1404, joerg.specht, Lars.Vogel, nikita, niraj.modi, patrick |
| Version: | 4.20 | ||
| Target Milestone: | 4.21 M1 | ||
| Hardware: | PC | ||
| OS: | Windows 10 | ||
| See Also: |
https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/182208 https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=7a8160aa2e7d115551b90536a89ab3bc7eddf204 |
||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 571909, 573985 | ||
Found this while during some research today: https://docs.microsoft.com/en-us/microsoft-edge/webview2/reference/win32/icorewebview2cookiemanager?view=webview2-1.0.818.41 The problem with Edge API is that they require a Browser instance to access cookies, while SWT API for cookie access methods are static, since the cookies are shared between all Browser instances in the application. (This is true for Edge and all other SWT Browser back-ends). I really don't want to create a hidden browser instance just for cookie access. There are several problems with this approach. Instead I think we can require that a Browser instance is created by the user before static cookie methods work, and the methods will fail if no instance is available. Any instance created with SWT.EDGE will do, it doesn't have to be dedicated or anything. Do you think this is an acceptable limitation? For all my use-cases this would work, I always have a browser instance before setting the cookies.
An possible alternative might be:
- if browser instance available, default behavior set cookie in browser
- if no browser instance available
-- storing the cookies in a store (maybe static):
--- List<String[]> cookies = new ArrayList<>();
--- cookies.add(new String[] {value, url});
--- if browser instance is created, set the cookies in the new instance
I didn't look far into the code, so this approach might have other obstacles.
I'm happy with your suggested solution, this is just a thought on more backward & cross-browser compatibility.
(In reply to Nikita Nemkin from comment #2) > The problem with Edge API is that they require a Browser instance to access > cookies, while SWT API for cookie access methods are static, since the > cookies are shared between all Browser instances in the application. (This > is true for Edge and all other SWT Browser back-ends). > > I really don't want to create a hidden browser instance just for cookie > access. There are several problems with this approach. Agree, we should avoid creating hidden browser instance for this use-case. > Instead I think we can require that a Browser instance is created by the > user before static cookie methods work, and the methods will fail if no > instance is available. Any instance created with SWT.EDGE will do, it > doesn't have to be dedicated or anything. Do you think this is an acceptable > limitation? +1 for above approach, that's also goes inline with the API contract of Browser#setCookie() method which is supposed to return: true if the cookie was successfully set and false otherwise (In reply to Flavio Donze from comment #3) > For all my use-cases this would work, I always have a browser instance > before setting the cookies. Mostly the browser instance will be present before setting the cookies. > An possible alternative might be: > - if browser instance available, default behavior set cookie in browser > - if no browser instance available > -- storing the cookies in a store (maybe static): > --- List<String[]> cookies = new ArrayList<>(); > --- cookies.add(new String[] {value, url}); > --- if browser instance is created, set the cookies in the new instance > > I didn't look far into the code, so this approach might have other obstacles. Having our custom cache for cookies may have it's own challenges. Even if we have a full-proof implementation, we might be considered breaking the API contract of Browser#setCookie() method which says: "The value is passed through to the native browser unchanged." So instead of passing we are caching them silently, so caching is not recommended until there is a real breaking use-case. Anyways, Thanks for the idea! New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/182208 Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/182208 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=7a8160aa2e7d115551b90536a89ab3bc7eddf204 (In reply to Eclipse Genie from comment #7) > Gerrit change > https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/182208 was > merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/ > ?id=7a8160aa2e7d115551b90536a89ab3bc7eddf204 Hi Nikita, You can still fine tune the JUnit changes & push it later on as a separate patch, if required create a separate bug. Thanks for this bug fix! Making verified Build id: I20210707-1800 using Edge 91 on Windows 10 |
Setting and gettting cookie values does work with integrated Internet-Explorer, the following code prints "123". Browser.setCookie("test=123", url); browser.setUrl(url); System.out.println(Browser.getCookie("test", url)); Switching to edge (-Dorg.eclipse.swt.browser.DefaultType=edge) will print "null" as result.