The code has some code styling issues:

--------------------------------------------------------------------------------
26s
57
FOUND 10 ERROR(S) AND 1 WARNING(S) AFFECTING 8 LINE(S)
26s
58
--------------------------------------------------------------------------------
26s
59
 100 | WARNING | Line exceeds 150 characters; contains 161 characters
26s
60
 115 | ERROR   | Please start your comment with a capital letter; found "//
26s
61
     |         | irelevant on new users"
26s
62
 116 | ERROR   | Expected "if (...)\n"; found "if (...) "
26s
63
 116 | ERROR   | Closing brace must be on a line by itself
26s
64
 117 | ERROR   | Please start your comment with a capital letter; found "//
26s
65
     |         | irelevant, because password was not changed by user"
26s
66
 118 | ERROR   | Expected "if (...)\n"; found "if (...) "
26s
67
 118 | ERROR   | Closing brace must be on a line by itself
26s
68
 119 | ERROR   | Please start your comment with a capital letter; found "//
26s
69
     |         | irelevant, because "resetting on pw change" is not activated"
26s
70
 120 | ERROR   | Expected "if (...)\n"; found "if (...) "
26s
71
 120 | ERROR   | Closing brace must be on a line by itself
26s
72
 149 | ERROR   | Please put a space between the // and the start of comment
26s
73
     |         | text; found "//$this->app->input->cookie->set($cookieName, '',
26s
74
     |         | 1, $this->app->get('cookie_path', '/'),
26s
75
     |         | $this->app->get('cookie_domain', ''));"
26s
76
--------------------------------------------------------------------------------
26s
77
UPGRADE TO PHP_CODESNIFFER 2.0 TO FIX ERRORS AUTOMATICALLY
26s
78
--------------------------------------------------------------------------------

please add your other changes to this pr. you can do that by going to this branch in your fork and editing the files there. they will then be added automatically to this pull request

Please follow XML coding style:
https://developer.joomla.org/coding-standards/xml.html

Do we really need an option for that? I would just go without it.
If you really want that parameter, the default values in code and in XML will have to match to prevent unexpected behavior.

Another thing to consider: You put the code into the "System - Remember" plugin, but that is actually just a proxy plugin so the real "Authentication - Cookie" plugin can do its work. So this code should go into that authentication plugin instead.

If we aren't importing authentication plugins in the MVC for CRUD actions then this is the right plugin.

How about we import the authentication plugins instead? I'd say that should be easy and could be useful for other authentication plugins as well. I don't think it's a good idea to spread the logic of the cookie plugin (again) over multiple ones. If we can, it should be kept within the cookie one.

The authentication plugins shouldn't be handling CRUD actions (since some people seem to be overly concerned about what tasks specific plugin groups actually listen to, apparently we should have a "User - Remember Me" plugin for this task if you buy into their thinking 😉 ).

The authentication plugins in general aren't treated the same as every other plugin group, there are actually only two places in core where they are actually loaded as "real" plugins otherwise the Authentication class pulls the plugin objects out of the dispatcher and direct calls the methods (so not using the event system). I would be hesitant to add more plugin based functionality to this plugin group since it is not treated as a correct plugin group (if we want to use it as a real plugin group the authentication system needs to be refactored to use proper events and not the crap logic it has now).

As far as where the code is, things are fine right now.

On the "user message": I may be "over sensible" in this respect, because of the underlying story why this pr exists...
If you think this user message is "clutter", I am not oposing this!

Do you guys think we need that option? Is there a reason you do not want to reset the cookies after a password change? Imho that "feature" should always be enabled. I don't see why someone would want to disable it. So the option could be removed to make the code simpler.

Agreed. Why would you want to enable a bad option

Do you guys think we need that option? Is there a reason you do not want to reset the cookies after a password change? Imho that "feature" should always be enabled. I don't see why someone would want to disable it. So the option could be removed to make the code simpler.

I was reading this from top, and was going to write that an option should not be added,
this behaviour should not be possible to disable !

is there anything left to be done before this PR gets merged back?

@schultz-it-solutions at least it needs 2 successfully Tests.

@Quy , @dgt41 , @ggppdk
any chance one of you guys can perform another test in order to finalize this pr?

I have tested this item ✅ successfully on f226416

@dgt41 , @ggppdk
any chance one of you guys can perform another test in order to finalize this pr?

I have tested this item ✅ successfully on f226416

Ready to Commit after two successful tests.

Merged thanks!