empty logout form makes formatting challenging

[sunnysideup]

Affected Version

silverstripe/recipe-cms                  4.2.2            
silverstripe/recipe-plugin               1.3.0              

Description

Log out form is empty. This is not great when you have formatting fieldset - e.g. a border or a background colour. As it looks weird with an empty form. There is only an hidden field in the fieldset. It is hard to target something like that with CSS (i.e. if fieldset has nothing in it that is visible then hide???)

Also the <div class="clear"><!-- --></div> seems a bit presumptuous. That is really something that people should add as they see fit themselves (I would personally use CSS to do a clear).

Maybe the logout button can be moved into the main fieldset.

<form id="LogoutForm_Form" action="/Security/logout?SecurityID=967e703a21024bac5f5809f48ad4d92c828d4826" method="post" enctype="application/x-www-form-urlencoded">

    <p id="LogoutForm_Form_error" class="message " style="display: none"></p>

    <fieldset>

            <input type="hidden" name="BackURL" value="http://google.com/" class="hidden" id="LogoutForm_Form_BackURL">

            <input type="hidden" name="SecurityID" value="967e703a21024bac5f5809f48ad4d92c828d4826" class="hidden" id="LogoutForm_Form_SecurityID">

        <div class="clear"><!-- --></div>
    </fieldset>

    <div class="btn-toolbar">

            <input type="submit" name="action_doLogout" value="Log out" class="action" id="LogoutForm_Form_action_doLogout">

    </div>

</form>

The same applies to the login form without backURL:

<form id="MemberLoginForm_LoginForm" action="/Security/logout?SecurityID=7aefc7e9d2c9cca8af13242fd75a4f8f9ac29d94" method="post" enctype="application/x-www-form-urlencoded">

    <p id="MemberLoginForm_LoginForm_error" class="message info">You're logged in as Default Admin.</p>

    <fieldset>

            <input type="hidden" name="AuthenticationMethod" value="SilverStripe\Security\MemberAuthenticator\MemberAuthenticator" class="hidden" id="MemberLoginForm_LoginForm_AuthenticationMethod">

            <input type="hidden" name="SecurityID" value="7aefc7e9d2c9cca8af13242fd75a4f8f9ac29d94" class="hidden" id="MemberLoginForm_LoginForm_SecurityID">

        <div class="clear"><!-- --></div>
    </fieldset>

    <div class="btn-toolbar">

            <input type="submit" name="action_logout" value="Log in as someone else" class="action" id="MemberLoginForm_LoginForm_action_logout">

    </div>

</form>

Steps to Reproduce

visit https://demo.silverstripe.org/Security/logout/ when logged in or visit https://demo.silverstripe.org/Security/login/ and log in.

Question

Rather than linking to https://demo.silverstripe.org/Security/logout/, should I use a the logout form directly in my code? Is there a link that bypasses this additional logout form altogether?

I am guessing I should use this as well:

https://github.com/silverstripe/silverstripe-framework/blob/4/src/Security/Security.php#L1341-L1345 and https://github.com/silverstripe/silverstripe-framework/blob/4/src/Security/Security.php#L1364-L1373

[kinglozzer]

As far as I’m aware, the <div class="clear"> etc is just the (antiquated) default form styling. You’re correct, you should be bypassing that form completely - in templates use {$LogoutURL}, or in PHP use Security::logout_url(), rather than hard-coding /Security/logout. The form is only there as a backup in case the CSRF token is missing (to stop attacks involving logging out the user without their knowledge) 😃

[sunnysideup]

Thank you for your quick response!

It may still be a good idea to fix the original bug lodged here. We should not have empty fieldsets (maybe hidden fields should go into their own area.

[robbieaverill]

We should not have empty fieldsets (maybe hidden fields should go into their own area.

Agreed, this doesn't do anything good for screen readers. Maybe putting the button into the fieldset would justify having it

[sunnysideup]

I see a couple of options:

1. put hidden fields in their own div or as direct children of form
2. add a class to the fieldset that indicates if there are any non-hidden fields
3. put hidden fields with actions

The second option would cause the least amount of disruption.