After using "Open Link in new window" the session is lost

[jmairboeck]

When using "Open Link in new window" for the first time after starting WebPositive, the session in Trac is lost. It seems that WebPositive is no longer sending the correct cookies to the server.

After relogin, the session stays, even after opening links in new tabs that way (as long as WebPositive is running).

[waddlesplash]

Yep, I've seen this for quite a long time, it's probably the most annoying thing in Web+.

[kallisti5]

Are cookies saved to disk in web+?

[pulkomandy]

Yes, when you exit Web+ they are written to the cookie jar (config/settings/Cookie) for non-session cookie (the session cookies are never stored of course). The file is an archived BMessage.

You can also use the "cookie manager" GUI in Web+ to check the cookies.

The likely cause here is a mixup it the way "sessions" are handled in WebKit side of the code. There is early work for having tabs in multiple sessions (for private browsing, etc) but supposedly there should currently be only one session (one BUrlContext) shared between everything. I suspect something is not done right and some tabs end up in different sessions.

[pulkomandy]

Quite likely a problem in WebKit side (I'd guess with the code to manage "sessions" and private browsing, which is half implemented).

[nephele]

Where is the relevant code in webkit?

[pulkomandy]

I suspect multiple instances of NetworkStorageSession are somehow created (when creating new tabs). It's unclear how these methods work together and what the expected flow is:

​https://github.com/haiku/haikuwebkit/blob/haiku/Source/WebKitLegacy/haiku/WebCoreSupport/FrameNetworkingContextHaiku.cpp#L77

​https://github.com/haiku/haikuwebkit/blob/haiku/Source/WebKitLegacy/haiku/WebCoreSupport/FrameLoaderClientHaiku.cpp#L1000

​https://github.com/haiku/haikuwebkit/blob/haiku/Source/WebCore/platform/network/haiku/NetworkStorageSessionHaiku.cpp#L58

There are also various functions to set cookies that are not implemented:

​https://github.com/haiku/haikuwebkit/blob/haiku/Source/WebCore/platform/network/haiku/NetworkStorageSessionHaiku.cpp#L124

We also didn't fully implement "private browsing" which would normally use a separate NetworkStorageSession instance (with a separate cookie jar, history, etc, that would not be saved to disk in any case). Normally this would use PAL::SessionID to identify a separate browsing session. I don't know what we're expected to do with that (in NetworkStorageSession constructor) and how much of it is already managed by WebKit. We should probably check if we get a single session or multiple ones.

[fatigatti]

The culprit seems to be the ​createWindow method in ChromeClientHaiku.cpp. This method is only called when the context menu "Open link in new window" is used. It is not called when clicking a link with the middle button, when using CTRL-T to open a new tab or even when using WebPositive's new window entry from the menu. There's even a FIXME in that method that warns about the importance of the frame to clone session information.

The difference between the different entities (Frame, Page, FrameLoader, ChromeClient) and how they interact it's not clear to me, but there seems to be a zillion ways to open a new page. On my setup, "Open link in new window" doesn't even do that, because the link is opened in a new tab instead. My first idea for a quick fix was to make that context menu entry use the same methods that somehow preserve session information (the ones used for middle clicking, Ctrl-T et al) but after a look it seems that is platform independent code and it's probably best to fix our createWindow.

We're possibly looking at a one-liner fix here, but I think we need to understand how this entities play togheter (and while we're at it, decide what to do with the context menu: fix current behaviour and open a new window, change the label to open in new tab, offer both choices, or whatever)

[pulkomandy]

The text in the menu entries is in cross platform code too so we have no control over the menu labels. There is a separate ticket for this but this will be redone when we move out of WebKitLegacy anyway.

And yes, this is very likely an one-line fix, we're llosing the session information in some place and so the new tab starts with a new session which doesn't contain the existing session cookies. That being said, maybe it's worth making a bit larger refactor and make sure the code to open new tabs or windows always follows the same path so there would not be so many cases to care about.

[kallisti5]

The loss of sessions is a pretty painful bug when you include the random crashes + usability issues.

Some sessions + cookies will be lost when the browser is closed, but if they were even a little more persistent it would likely help the frustrating "login, do stuff, crash, login, do stuff, lockup, etc" loop WebPositive users can fall into. Tabs "locking up" happens a lot still unfortunately.

[pulkomandy]

The loss of cookies on crash is yet another thing and very simple to explain: the cookie jar is written to disk on exit. If there is a crash, it is not written.

The lock up is also, in most cases, due to the very bad code for media handling. Whenever a site tries to play a sound or video, the whole browser freezes until the file is completely downloaded. This is a problem in the media kit and/or the way WebKit uses it when creating a BMediaFile from an HTTP URL.

[fatigatti]

Found it :)

I left a pull request on ​https://github.com/haiku/haikuwebkit/pull/13 (is that the official repo for HaikuWebkit BTW or is there another channel to submit patches?)

[nephele]

That is the right channel, if you search for haikuwebkit on the web you might find my private fork still though. :)

[pulkomandy]

Is this fixed by the switch to curl?

[jmairboeck]

Looks good to me after a quick test.

[waddlesplash]

Huzzah!