As commented in #18569:
I tried to reproduce the issue for a while, but wasn't able to with my normal setup. So I started to have some deeper thoughts about this issue. I excluded a possible browser bug, so it needed to be something within Matomo that let's the browser struggle with the session cookie.
So I took a closer look at the requests that are sent by the Overlay. All requests that are sent to
index.phpare actually setting the session cookie. But there is one big difference those requests are having.
We have a special handling for the
samesitecookie attribute implemented for all requests that are going to the Overlay module:
The overlay page is sending multiple requests to the API. Such requests are actually setting the value to
lax, while the requests going to the overlay module are setting the value to
That wasn't a problem in my setup as the tracked url and matomo were both running on the same domain. So I set up another vhost to have different urls for matomo and the tracked site. Using this set up it was easy to reproduce the problem.
So the problem actually is that depending on the request that last finished, before the
notifyParentIframerequest is sent, the samesite attribute is set to
noneeverything works fine, but if the attribute is
laxthe iframe can't access the cookie, so it is not sent with the request, causing a new session to be started.
This fix actually checks if the referrer contains
module=Overlay to set the
samesite=none for those requests as well. This should properly work unless the referrer gets blocked for some reason.
An alternative would be to send some kind of magic parameter with all requests that are sent by the overlay, so we can check for that parameter.
While testing that I also saw, that the Overlay was sending a request to
API.getPagesComparisonsDisabledFor, which is not needed, so I added the Overlay module to the exclude list for that.
@tsteur I have targeted the
next_release branch as it might be nice to include that fix maybe. If you think it's to risky, feel free to change the milestone and I'll rebase the branch to
@sgiehl seems various other commits are included here? As the release might be today have to maybe target 4.x-dev after all?
@tsteur Sorry I merged in 4.x-dev without thinking much about it. Rebased the branch so the changes aren't included anymore. Feel free to merge if needed or change the milestone and I can rebase to 4.x-dev again
@tsteur changed the base to 4.x-dev. Should be ready for a last review now....
Looks good to me from a code perspective. @bx80 not sure if you can still easily reproduce the issue locally. If so, could you maybe also confirm it works?