Logout confirmation form shows inappropriate confirmation description

Prashant.c → made their first commit to this issue’s fork.

I agree with this, this message is kind of irrelevant here because we already have in bolds Are you sure you want to log out? and this is not something that cannot be undone, the user can always log in again.

I would say a better thing would be to just not have this description at all.

Prashant.c → changed the visibility of the branch 3469116-new-user-logout to hidden.

In the original issue 🐛 User logout is vulnerable to CSRF Fixed also the description is not shown https://www.drupal.org/files/issues/2024-03-26/Screenshot%202024-03-26%2... → .

Tests are failing because the getDescription() {} method is expecting a translatable string, which in case we have to provide. Looking for input here.

This will need a title update as just the word "misleading" is vague and not sure what the problem is.

Probably should have test coverage as well.

Before adding tests we need to decide on what would be the "description" for this as requested in #8 🐛 New User Logout confirm Form is misleading Needs work

sahana _n → made their first commit to this issue’s fork.

Hi Prashant.c

I created an MR Please review it.
If there are any inaccuracies, I would greatly appreciate your suggestions for improvement.

Thank you!!

@sahana _n

Thanks for your efforts but in the existing MR we are already using the getDescription() {}new MR was not required. Please see #8 🐛 New User Logout confirm Form is misleading Needs work

prashant.c → changed the visibility of the branch 3469116-Logout-confirmation-form-shows-inappropriate-confirmation-description to hidden.

@smustgrave The Title of the issue has already been updated removing the tag. Please re-add if you still see some issues with the current title.

TBD. Suggesting a couple of descriptions we can use:

• Logging out will end your current session, and you will need to log in again to continue.
• Once you log out, you’ll need to sign in again to access your account.

I like the second option, for brevity.

Hi,

Even I liked the second option and I updated the MR Please review.
I verified that in Drupal version 11.x.

I would greatly appreciate your suggestions for improvement. Please let me know.

Thank you!!

Just wanted to confirm this one because as noted above, this text wasn't there when the change was committed. Added a screenshot to the IS.

I also agree that it's better without a description, I think at this point logging out is pretty self explanatory!

@pameeela wrong screenshot attached :). Without description is good but see #8 🐛 New User Logout confirm Form is misleading Needs work , we have to provide the description.

Oops! Fixed.

I realise that the tests failed without a description, but that doesn't mean it's not possible :)

I guess it is better to update it to something more accurate for now than to have it be confusing.

Yes, and this is added in #19 🐛 New User Logout confirm Form is misleading Needs work

Yes, I know that :)

I have made a minor change to the wording for review.

Looks good to me. In my opinion, we can move it to RTBC.

Updated screenshot. Still needs a test.

Added the check to test the existence of the description and that should be sufficient I guess.

Hi, I have verified this issue in our D11 version with the latest MR- 3469116-11.x-new-user-logout and it applies cleanly. This isssue is resolved now.

Testing steps:
As a logged in user, directly visit user/logout on a site (to trigger the CSRF protection and use the confirm form). The misleading text will be visible above the confirm/cancel buttons.

Test Results:
Logout confirmation form shows the appropriate confirmation description now.

Adding before and after screenshots.

Still putting this issue in needs review for the further code reviews.

Thanks!

Tests Failed.

Hi @dishakatariya we already have screenshots for this so we don't need any new ones.

Thanks, @pameeela for the rebase, pipelines passed, moving to NR.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

False positive

Hello,
I have verified this issue on D11 version.
MR applies cleanly.

Testing steps:
As a logged in user, directly visit user/logout on a site (to trigger the CSRF protection and use the confirm form).
The misleading text will be visible above the confirm/cancel buttons.

Test Results:
Logout confirmation form shows the appropriate confirmation description now.

Refer screenshots-

Everything looks good, I'm sure the messaging will always be of debate, but this could always turn to a config option at a later date.