Problem/Motivation

In Chrome, I'm seeing an issue where Drupal's "log in" submit button and "log out" link sometimes do not do anything when the PWA module is enabled. Submitting the login form results in a page submission that acts like it is loading, but never completes. Clicking the log out links is similar, the browser tab shows activity but nothing happens. Clicking the log out link a second time works.

Steps to reproduce

Submit the Drupal log in form or click the log out link when logged in.
Was able to reproduce in Chrome 97 and 101 on Linux. I do not see this issue in Chrome 83 on Linux. Firefox does not seem to be affected.

Proposed resolution

Chrome seems to be doing something funny when it receives the "Clear-Site-Data" header in a response. Commenting out the $response->headers->add() lines in \Drupal\pwa\EventSubscriber\ResponseSubscriber reliably fixes the issue for me. Unfortunately I don't have an alternate solution.

CommentFileSizeAuthor
#33 2023-05-16 10_15_13-Home _ Drush Site-Install.png2.77 KBgrevil
#33 2023-05-16 10_13_21-Home _ Drush Site-Install.png2.76 KBgrevil
#17 3279305.patch898 bytesranjit1032002

Issue fork pwa-3279305

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bgreco created an issue. See original summary.

djdevin’s picture

djdevin
he/him
Primary language English
Location Philadelphia
commented

I am seeing the same issue with the D7 version. Removing the calls to drupal_add_http_header() on login/logout makes it work.

The particular use case was with https://www.drupal.org/project/passwordless to launch a user from an email link into the PWA as a logged in user. It hung on the login but the user was logged in on the subsequent refresh.

zenimagine’s picture

zenimagine commented
Related issues: +#3321187: 403 error with user account

I confirm the problem and it is a serious problem, because it is also with the form of resetting the password and creating the password of a new account.

It is not possible to create a new password when the module is activated. Do you have a patch? emergency. Thanks

zenimagine’s picture

zenimagine commented
Priority: Normal » Critical
rupl’s picture

rupl
he/him
Primary language English
commented
Priority: Critical » Normal

Before we label this a critical bug, can anyone confirm that setting excluded URLs are totally ineffective in working around the bug?

bgreco’s picture

bgreco commented

Yes, I was seeing the problem even with user/.* in the exclude urls.

zenimagine’s picture

zenimagine commented

I confirm that excluding URLs is ineffective. I passed in criticism because with this active module, anonymous users can no longer register and authenticated users can no longer use forgotten password

zenimagine’s picture

zenimagine commented

Do you need a temporary patch urgently? I am completely stuck because of this problem. My users can no longer register on my site and create a password

zenimagine’s picture

zenimagine commented
Priority: Normal » Critical
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue tags: +Needs tests
verper’s picture

verper commented

bug still exists

luisasa’s picture

luisasa commented

We also detect the problem in Chrome, in IE, in Opera.
Drupal core 9.5.5.
Both in login or logout the system runs empty. To resolve the user must click on a link and quickly manages to enter.
Also note an excessive slowdown of the system when loading pages, views ...
Is it possible to help us to solve the problem?
Thank you

kris77’s picture

kris77 commented

Same issue too...

umit’s picture

umit
Primary language Turkish
Location Turkey
commented

I'm having the same problem +1

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Sorry guys, I don't have the time to write a patch currently. If anyone has the time, please do it, including tests and I will review it quickly!

Ranjit1032002 made their first commit to this issue’s fork.

ranjit1032002’s picture

ranjit1032002
Primary language English
Location India
commented
Status: Active » Needs review
StatusFileSize
new 3279305.patch898 bytes

Created a patch for the issue mentioned, please review.
Thank You.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

Well that doesn't really help. It's just what has been written in the issue summary. What we need is a clean fix, explanation why it was used before, but should (perhaps?) not be used anymore and what are the consequences.

The result of #17 is just commented out code with unclear results from my perspective. Anyway I'll prepare a MR now which removes these lines to be used as workaround and have a look if I can find a blame entry where it was added ...

Anybody opened merge request !49

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

These lines were added here: #3078282: Service Worker caches admin toolbar and contextual links

So it seems this is a regression. I'll ask the users from that issue to have a look please.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Related issues: +#3078282: Service Worker caches admin toolbar and contextual links
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

I'm wondering if perhaps the (double) quotes may cause this?
'"storage"' vs, 'storage' as string value?

In Drupal 7 the values also had these double-quotes: https://git.drupalcode.org/project/pwa/-/blob/7.x-2.x/pwa.module but looks really strange.

Could someone try that? I prepared MR!50 with that change for a test. As patch: https://git.drupalcode.org/project/pwa/-/merge_requests/50.diff - does that change something?

Anybody opened merge request !50

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs work » Needs review
Issue tags: +Regression
kris77’s picture

kris77 commented

This patch https://git.drupalcode.org/project/pwa/-/merge_requests/50.diff works for me

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks @Kris77 interesting to hear! That sounds good and if it also works for others, we should have a deeper look at the headers sent and if they work as expected (clearing those caches)!

arunkumark’s picture

arunkumark
Location Coimbatore
commented

Verified the patch MR!50 it resolve the issue of user login and logout.

+RTBC

phannphong’s picture

phannphong commented

I don't face this issue when user logout but face the exactly issue when user login.
The patch MR!50 is solved my problem when user login.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Could someone please compare and debug the mentioned Clear-Site-Data header with and without this patch?

We need to be totally sure that this is the correct solution, before merging this. I don't have much time currently, sorry.

And finally a test for this header should be added, so we can be totally sure this doesn't break again.

grevil’s picture

grevil commented

I couldn't recreate this issue under windows using Chrome Version 113.0.5672.93 (Official Build) (64-Bit) and Olivero 10.0.9.

Although the patch in MR 50 makes sense... @Anybody, how do you want to proceed here?

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@Grevil: Please see #22 could you check these header values on the login page please with and without the patch?

My guess, which the patch is based on, was, that the quotes are unexpectedly output with the string, which breaks the functionality.

iamdroid’s picture

iamdroid commented

Hi folks, thanks for working on this.

I've tested MR!50 - it works!

+1 for RTBC.

grevil’s picture

grevil commented
StatusFileSize
new 2023-05-16 10_13_21-Home _ Drush Site-Install.png2.76 KB
new 2023-05-16 10_15_13-Home _ Drush Site-Install.png2.77 KB

Headers without patch:
screenshot

Headers with patch applied:
screenshot2

grevil’s picture

grevil commented
Status: Needs review » Reviewed & tested by the community
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@Grevil: Thank you sooooo much, so my assumption was correct :D Whao!

Let's merge it!

Anybody closed merge request !49

  • Anybody committed dbf8d5c6 on 8.x-1.x 
    Issue #3279305 by Anybody, Ranjit1032002: Log in / log out problems in...
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Reviewed & tested by the community » Fixed
grevil’s picture

grevil commented
Version: 8.x-1.x-dev » 2.x-dev
Status: Fixed » Needs work
grevil’s picture

grevil commented
Status: Needs work » Reviewed & tested by the community

  • Anybody committed a6677404 on 2.x 
    Issue #3279305 by Anybody, Ranjit1032002: Log in / log out problems in...
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Reviewed & tested by the community » Fixed

Cherry-picked to 2.x and created 8.x-1.7 and 2.0.0-rc2 with this fix included! Thank you all!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.