One very important correction. You removed the onclick element from the array that builds the button. **Please do NOT do that! ** This is meant to be usable by third party plugins which implement non-password logins such as but limited to Single Sign-On with external SSO servers (e.g. Amazon AWS SSO), Single Sign-On with JavaScript-based login (e.g. Synology SSO), social network logins (e.g. Login with Facebook) and future-proofing Joomla for similar core solutions to WebAuthn without the need to change the core in a backwards incompatible manner.

@nikosdion So shall I just use $onClick = ''?

The onclick code has been moved to JS. If it needs to be kept in the markup, then something is wrong here.

@C-Lodder I explained it more in depth in my code review. It's not about WebAuthn working – your solution works great – but making it possible for third parties to be able to reliably implement non-password logins. Single Sign-On is an essential enterprise / corporate feature and there is active interest in it; people have approached me asking to implement that in Joomla 3 on the basis that it's similar to what I'm doing with SocialLogin.

My feeling from Forum For The Future in Spain earlier this year is that Joomla is trying to be friendlier to enterprise users (makes sense; that's where the money, therefore the corporate volunteer time, will come from). So these 30 or so characters can make a big difference for our future.

@nikosdion Sorry, only just saw your actual code review comments now. I've reverted the main onclick removal which generated the login buttons. I'll keep the other ones in the layouts (edit/delete label) removed unless you feel they need to be reverted too.

@C-Lodder Your change is good. That's the only place I think needed change. Backend code is fully owned by the plugin, there's no link to the changes in mod_login. I'm happy :)

Looks good at a quick review. Can someone just do a quick test of functionality and good to merge

@nikosdion @dgrammatiko Data attributes are now in place

Since I was mentioned here I guess I need to provide some answers, so:
@nikosdion you're right I'm thick on my approach about communicating the messages but this was on purpose: the more severe the comment, more chances people will react on it. The only problem with that approach was that I was getting the wrong reactions, people just commented on the messenger without paying attention to the actual message. Nevertheless it worked out in some occasions: CSP, Accessibility, etc, but it failed miserable on others: modularity, decoupling, etc...

@HLeithner you are right about CSP and that was one of the driving forces to remove all the inline listeners but there is one more reason: reducing the vector for XSS attacks. I'm not sure if this was ever written in any of the production team meetings but there was a consensus on that some years ago IIRC.

And now the part that @C-Lodder will gonna hate me for ?, couple more suggestions:

• According to https://caniuse.com/#search=webauthn WebAuthn is relatively new (the newest browser that added support for it was Android Browser, late Feb 2020) so it will be wise to have some feature detection here, eg: https://stackoverflow.com/questions/55866575/how-to-detect-if-the-browser-has-support-for-webauthn
• Also another fact is that there won't be support for webAuthn without support for ES Modules. This give us some flexibility, so we can just load the ES6 ONLY file with a tag type=module. Also doing it so we can remove the document listener for DOMContentReady as all modules are deferred by default. Also you could safely remove the IIFE as modules have specific scope (simplified: the variables, etc won't be global, are already contained in the module's scope)
• Finally to cater for the older browsers, or the evergreen without support I propose to go the progressive enhancement way here. Plain English: in the (front end facing) layouts add by default an attribute disable on the button and then in the ES6 file, once we executed the feature detection part and we're sure that the feature is available remove it (eg btn.removeAttribute('disable'). This will also take care of the users that have disabled JS, so for all these the button will not be clickable. This can be enhanced even more with some message, but I think in order to do so we need to do some work on the build tools. Right now they just transpile every ES6 file, in this case this is not what we want (there is no reason to deliver the full script as it will never be executed in a legacy browser). To remedy this I think we need to introduce something in the settings.json, an array for excluding files from been transpiled to ES5. Once that is in place we can add an .es5.js that will just append/unveil the message (eg webAuthn is not available in you dinosaur, use a modern browser)

can you fix the conflicting file please

I'm not against what @dgrammatiko said actually - i'm definitely in favour of progressive enhancement - but I'd like to see this merged as a first step on the path to greatness :) - this es6 refactor is required either way (even if it can be cleaned up a bit further when we move to modules) So that being said can we please test this?

Conflicts resolved

Is this going anywhere?

@C-Lodder please don't close this, it's highly needed to be merged

(sorry I can't test right now)

I feel a bit un-easy merging this without tests - but I guess there's a better chance of testing if it's in core :/