Opened 3 years ago

Closed 6 months ago

#15625 closed bug (fixed)

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

Reported by: jmairboeck Owned by: pulkomandy
Priority: high Milestone: R1/beta4
Component: Kits/Web Kit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking: #13699, #15235, #16460, #17933
Platform: All


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).

Change History (20)

comment:1 by waddlesplash, 3 years ago

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

comment:2 by waddlesplash, 3 years ago

Blocking: 13699 added

comment:3 by waddlesplash, 3 years ago

Blocking: 15235 added

comment:4 by kallisti5, 3 years ago

Are cookies saved to disk in web+?

comment:5 by pulkomandy, 3 years ago

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.

comment:6 by waddlesplash, 3 years ago

Blocking: 16460 added

comment:7 by pulkomandy, 2 years ago

Component: Applications/WebPositiveKits/Web Kit

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

comment:8 by nephele, 15 months ago

Priority: normalhigh

comment:9 by nephele, 15 months ago

Where is the relevant code in webkit?

comment:10 by pulkomandy, 15 months ago

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:

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

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.

comment:11 by waddlesplash, 9 months ago

Blocking: 17933 added

comment:12 by fatigatti, 9 months ago

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)

comment:13 by pulkomandy, 9 months ago

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.

comment:14 by kallisti5, 9 months ago

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.

comment:15 by pulkomandy, 9 months ago

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.

comment:16 by fatigatti, 8 months ago

Found it :)

I left a pull request on (is that the official repo for HaikuWebkit BTW or is there another channel to submit patches?)

comment:17 by nephele, 8 months ago

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

comment:18 by pulkomandy, 6 months ago

Is this fixed by the switch to curl?

comment:19 by jmairboeck, 6 months ago

Looks good to me after a quick test.

comment:20 by waddlesplash, 6 months ago

Milestone: UnscheduledR1/beta4
Resolution: fixed
Status: newclosed


Note: See TracTickets for help on using tickets.