? Pending

User tests: Successful: Unsuccessful:

avatar infograf768
infograf768
29 Nov 2017

See #18863

This patch should solve 2 issues (tested in Firefox Quantum)

  1. Displaying the email field after the username and before the password field makes the browser proposes passwords instead of mails in that field. The order was always correct in 3.x.

2. Autocomplete set to off is broken for the passwords fields because of the new js with an "eye" icon letting display in clear the password.
I.e. for example when user registers but also when creating a new user in backend or editing profile.
eye

Summary of Changes

  1. Installation and creating new user in backend: changing order of fields to normalizes that order as already existing for registration where the issue is not present:

Existing Registration:
registration

After patch

Back-end (the grey bar is not created by this PR):

backendusercreation

Installation:

installation

2. Correcting autocomplete off for passwords
In 3.x the code was
<?php // Disables autocomplete ?> <input type="password" style="display:none">
But as I guess the js to hide/show the password depends on the type, we just have to use instead
<input autocomplete="off" style="display:none"> when the field type is password
After patch, this is corrected in Installation, registration, creating new user in back-end and profile edit.

Testing Instructions

Installation:
Just install with Firefox using an already used/saved username that was recorded by Firefox.
I used "admin"
Test before patch (screenshots already available in #18863
After patch:
screen shot 2017-11-29 at 10 37 46
screen shot 2017-11-29 at 10 41 49
emailfield

Test display or not of password in clear when clicking on the "eye" icon

Registration
Allow registration by user in User Manager Options.
Create a Registration menu item in frontend.
(if you want to use Module Login form, you still have to create a menu item as Trying to register from the module is for now broken)

test before and after patch.

Test also editing profile and creating user in back-end before and after patch.

@Webdongle @dgt41

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar infograf768 infograf768 - open - 29 Nov 2017
avatar infograf768 infograf768 - change - 29 Nov 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 29 Nov 2017
Category Administration com_users Front End Installation
avatar infograf768 infograf768 - change - 29 Nov 2017
Labels Added: ?
avatar Webdongle Webdongle - test_item - 29 Nov 2017 - Not tested
avatar Webdongle
Webdongle - comment - 29 Nov 2017

I have not tested this item.

Tested Full Success


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18911.

avatar infograf768
infograf768 - comment - 29 Nov 2017

What is incorrect? It does prevent autocompletion as stated.
The browser still proposes possible values but it DOES NOT autocomplete.
Please explain what is wrong.

avatar mbabker
mbabker - comment - 29 Nov 2017

Introduces major accessibility issues by using hidden input fields for starters.

avatar brianteeman
brianteeman - comment - 29 Nov 2017

Read the link I posted and then you will see why. It is written in plain English

avatar brianteeman
brianteeman - comment - 29 Nov 2017

For those who don't want to click a link to read.

many modern browsers do not support autocomplete="off" for login fields:

If a site sets autocomplete="off" for a form, and the form includes username and password input fields, then the browser will still offer to remember this login, and if the user agrees, the browser will autofill those fields the next time the user visits the page.
If a site sets autocomplete="off" for username and password input fields, then the browser will still offer to remember this login, and if the user agrees, the browser will autofill those fields the next time the user visits the page.
This is the behavior in Firefox (since version 38), Google Chrome (since 34), and Internet Explorer (since version 11).

avatar dgt41
dgt41 - comment - 29 Nov 2017

This is the behavior in Firefox (since version 38), Google Chrome (since 34), and Internet Explorer (since version 11).

Also same for Safari. So basically ALL major browsers. I guess we shouldn't cancel default browsers behaviour, either we like it or not!

avatar brianteeman
brianteeman - comment - 29 Nov 2017

You can of course change the firefox behaviour and tell it that you want it to obey the autocomplete setting by doing the following

  1. In a new tab, type or paste about:config in the address bar and press Enter. Click the button promising to be careful.

(2) In the search box above the list, type or paste signon and pause while the list is filtered

(3) Double-click the signon.storeWhenAutocompleteOff preference to switch it from true to false

avatar infograf768
infograf768 - comment - 29 Nov 2017

Introduces major accessibility issues by using hidden input fields for starters.

that one from @mbabker may make sense. I am not a specialist of accessibility. Then why not modify it in 3.8 as we use there a similar code and this for ages?

@ others my test here show, for these specific pages, that Firefox does NOT autofill after saving the password. please just test.

in any case, if we do not merge this or something similar, it will autofill and therefore we will never have autocomplete off for joomla users.

but maybe that’s what some people want.

avatar dgt41
dgt41 - comment - 29 Nov 2017

@infograf768 autofill is irrelevant in the installer!

avatar brianteeman
brianteeman - comment - 29 Nov 2017

Please note that password managers also do what browsers do and completely ignore the autocomplete=off setting on a password field - for obvious reasons

Your pr is just exploiting a "bug" that if the field before the password is empty then the browser doesnt know what to autofill the field with.

avatar brianteeman
brianteeman - comment - 29 Nov 2017

The only valid place to disable autofill of the username and password is in the administrator usermanager

This can probably be achieved by simply giving the field a default value of space

  	default=" "
avatar brianteeman
brianteeman - comment - 29 Nov 2017

From Microsoft in 2009

You might wonder: "Why does IE11 ignore the AUTOCOMPLETE attribute on login forms while continuing to respect the attribute on all other AutoComplete input elements (e.g. address, credit cards, phone #, etc)?"

The answer is that Password Managers improve real-world security, and the IE team felt it was important to put users in control. Users rely on their password manager to permit them to comfortably use strong passwords. Password managers encourage strong, unique password creation per site, but unique, strong passwords are often difficult to remember and type on touch devices. If the browser doesn't offer to autocomplete a password, the user assumes that the browser is broken. The user will then either use another browser that ignores the attribute, or install a password manager plugin that ignores it. I'm an example of this myself-- until IE11, I used a weak password on the site that publishes my book because I couldn't convince them to remove the AUTOCOMPLETE attribute from their login form. Now, with IE11, that worry is gone and I can use a strong password on that site without a major hassle.

source: https://blogs.msdn.microsoft.com/ieinternals/2009/09/10/why-wont-ie-remember-my-login-info/

avatar Webdongle
Webdongle - comment - 29 Nov 2017

If the user chooses to save the the form details then they save email/password not admin password.
Try this and see
Usenamer: anewuser
email: me@xxx.yyy.com
Password: qwerty

Save the form when prompted

Once installed you can not select autofill for
Usenamer: anewuser
Password: qwerty

The issue here is not how the autofill operates but the positioning of the fields in the form. At the moment the email field is paired with the password field and thus is being recognised as (and treated as) the username field.

Having the browser recognise the email field as the username field (that matches the password) is incorrect behaviour. By the user saving that combination they will have not autosaved the correct combination to the Joomla login.

The patch rectifies the problem caused by placing the fields correctly.

avatar Webdongle
Webdongle - comment - 29 Nov 2017

Problem also exists in create new user ... the email is parsed to the username field of the autofill

new user 01

avatar infograf768 infograf768 - change - 30 Nov 2017
Title
[4.0] Correcting password autocomplete and email field considered as a password field
[4.0] Correcting email field considered as a password field
avatar infograf768 infograf768 - edited - 30 Nov 2017
avatar infograf768 infograf768 - change - 30 Nov 2017
The description was changed
avatar infograf768 infograf768 - edited - 30 Nov 2017
avatar infograf768
infograf768 - comment - 30 Nov 2017

I have now deleted all code implementing autocomplete to off.
Therefore, this PR just reorders where necessary the passwords and email fields.

Note: If autocomplete offshould never be used in joomla, I suggest to take off all code related to that in core: xml, field library, etc. I let this to those who know better.

Note 2: I still consider not setting autocomplete to off for new registration, creating new user in back-end and profile edit is a mistake as long as we let autofill to yes for login, which is the case with the PR.

avatar Webdongle
Webdongle - comment - 30 Nov 2017

@brianteeman
You confusion arises because you think the name of the field defines the username stored in the login autofill for the browser. When in fact it is the field type=password that defines the field that defines the username stored in the login autofill for the browser.
https://www.weblinksonline.co.uk/testing/testinstall.htm

avatar brianteeman
brianteeman - comment - 30 Nov 2017

I am not installing flash for anything

avatar infograf768
infograf768 - comment - 30 Nov 2017

No need for flash.

Here is a screenshot before this patch:
screen shot 2017-11-30 at 17 16 30

Anyone who does not understand needs serious help.

avatar brianteeman
brianteeman - comment - 30 Nov 2017

At no point in the OP do you ever say anything about the email address being stored as the username

avatar infograf768 infograf768 - change - 30 Nov 2017
Title
[4.0] Correcting email field considered as a password field
[4.0] Correcting email field considered as a username field
avatar infograf768 infograf768 - edited - 30 Nov 2017
avatar infograf768
infograf768 - comment - 30 Nov 2017

Modified title to fit indeed. But @Webdongle explained it a lot

It was only correct in the sense that it was "proposing" passwords to fill the email field, which is a consequence of the wrong order.

avatar brianteeman
brianteeman - comment - 30 Nov 2017

Is anything in your OP correct? Everything in the OP seems different to that commented on by @Webdongle

avatar infograf768
infograf768 - comment - 30 Nov 2017

See #18911 (comment)

and please, let's not discuss this anymore (we call this in French Enculer les mouches)

You now know the problem and why we need to reorder the fields. Please test and mark the Test OK so that we can move on.

avatar Webdongle
Webdongle - comment - 30 Nov 2017

@brianteeman

At no point in the OP do you ever say anything about the email address being stored as the username

I did tell you ages ago #18863 (comment)
And #18863 (comment) but you refused to listen and locked that topic

it is irrelevant to your issue. your issue is your browser as explained.
I am now locking this

@infograf768 is saying the same as me but you are not seeing it.

When there is a password field the browser treats the field (immediately) in front of it as the login Password field. Consequently it offers to save the contents of the password field and the one field (immediately) in front of it as the login username field.

Thus with fields in the default.php
getLabel admin_user ... Type text
getLabel admin_email ... type email
getLabel admin_password ... type password

The field with getLabel admin_user is just a text field to the browser. The confusion is those who (mistakenly) think that because a text field has a getLabel admin_user that the browser autofills it with stored username data

The field with getLabel admin_email the field (immediately) in front of it the login Password field is stored as username by the browser. Thus the browser is using the getLabel admin_email as the field that contains the username which is to be saved for login purposes.

Their confusion arises because they think the name of the field defines the username stored in the login autofill for the browser. When in fact it is the field type=password which defines the field that defines the username stored in the login autofill for the browser.

avatar mbabker
mbabker - comment - 30 Nov 2017

At no point has it been communicated in this way. Everything before the last hour basically came across as a "the browser autocomplete is broken so we need to disable it because I find this confusing" commentary.

avatar infograf768
infograf768 - comment - 30 Nov 2017

@mbabker
this is because the original error (order of these fields) is masking the wrong proposals to fill the field.
In a way or another (not in the login forms evidently), we have to correct and set autofill to false to avoid getting automatically a passord\mail we used for another site when using same username. It is specially important on localhost. My solution may not have been the right one, but we need one.

avatar mbabker
mbabker - comment - 30 Nov 2017

we have to correct and set autofill to false to avoid getting automatically a passord\mail we used for another site when using same username

That is not our issue to fix (we are not going to be messing with browser behaviors). As long as we are setting form fields in the correct order we are doing our due diligence and anything further is going beyond what should be a reasonable scope of responsibility.

(And for what it is worth, password manager and autofill type stuff have logic to function based on the domain you're on, if I stored all of my joomla.org passwords under the root domain then I would have to manually select the right one for each subdomain but because they are correctly stored under a subdomain then I am only prompted the credentials for that specific site)

avatar Webdongle
Webdongle - comment - 30 Nov 2017

@mbabker

At no point has it been communicated in this way. Everything before the last hour basically came across as a "the browser autocomplete is broken so we need to disable it because I find this confusing" commentary.

Eight days ago said it was the order of the fields that was the problem. It was Brian that turned the focus to the autofill

As long as we are setting form fields in the correct order we are doing our due diligence and anything further is going beyond what should be a reasonable scope of responsibility.

(And for what it is worth, password manager and autofill type stuff have logic to function based on the domain you're on, if I stored all of my joomla.org passwords under the root domain then I would have to manually select the right one for each subdomain but because they are correctly stored under a subdomain then I am only prompted the credentials for that specific site)

That is the point ... the autofill is being told to use the getLabel admin_email ... type email field as the Username in the Login screen. So it saves the email address as the username instead of the username. The default.php has the fields in the wrong order. The getLabel admin_email ... type email field is where the getLabel admin_user ... Type text should be.

avatar mbabker
mbabker - comment - 30 Nov 2017

*sigh*

We are not telling the autofill system what to use. The browser is determining that, as you have been so keen on pointing out. So yes, it is a browser limitation as it relates to autofill behaviors that we are patching around by changing our UI. It is NOT a Joomla bug. There is no "wrong order" as it relates to form fields (as far as data structure goes, UX and workflow is another thing altogether).

avatar Webdongle
Webdongle - comment - 30 Nov 2017

@mbabker

We are not telling the autofill system what to use. The browser is determining that

Yes we are because the browser is 'determining' the value to be saved as the Login username because the default.php has the getLabel admin_email ... type email directly above the getLabel admin_password ... type password

avatar dgt41
dgt41 - comment - 30 Nov 2017

How about joomla follows common logic?
How about username === email address as the rest of the web?

avatar brianteeman
brianteeman - comment - 30 Nov 2017

whoever knew that people other joomla might have a bug in their software

avatar mbabker
mbabker - comment - 30 Nov 2017

:eyeroll:

We are not communicating this to the browser. The browser is inferring it. End of story. There is no magical markup syntax that says "this field should be used as a username for autofill".

To be crystal clear. This patch, while it might fix an undesired behavior, IS NOT A BUG IN JOOMLA. Go yell at Mozilla if you don't like the way their browser's autofill system makes inferences. I suggest testing the autofill systems of other browsers or password managers and lodging the same complaint with them.

avatar brianteeman
brianteeman - comment - 30 Nov 2017

https://stackoverflow.com/questions/10738090/why-firefox-autocomplete-even-with-different-input-name/10745884#10745884

So firefox does NOT do what it should do and store the field with the username - it tries to be clever and ignores the standards and saves the field before a password

avatar Webdongle
Webdongle - comment - 30 Nov 2017

We are not communicating this to the browser. The browser is inferring it. End of story. There is no magical markup syntax that says "this field should be used as a username for autofill".

Yes we are because we know that the field above the type=password is a 'reserved position' . It is 'Reserved' by the browser for storing the username. By putting the email field in the position that the browser uses to autofill Login Username you are supplying the browser with the wrong data to store.

There is no logic in separating the fields (that store the) Username and Password data and every reason to place the field (that stores the) Username data in a Position (that browsers reserve) for a Username.

avatar mbabker
mbabker - comment - 30 Nov 2017

I'm too stressed for this crap to keep going around in meaningless circles, and unless this project is going to pay for the hospitalization I am on my way toward from excessive stress, anxiety, and unhealthy weight loss, I am unsubscribing after this comment from this thread before I really do lose it this time.

You show me where in the HTML standard or ANY browser structure that there is a definition to say "this field should be used for the username in autofill". I will eat my own words if you can find the existence of such a standard. Until that exists, Joomla CANNOT communicate what you are saying it does. As I said, and as has been proven by your own actions, the browser is INFERRING this. AGAIN, NOT A JOOMLA BUG

avatar Webdongle
Webdongle - comment - 30 Nov 2017

I'm too stressed for this crap to keep going around in meaningless circles,

So am I. When J4 is out and users cant login because email/password was (as a result of your refusal to accept default.php has the fields in the wrong order) saved ... then there will be a great big Told you so. One last time to try and explain where the error in your logic is. By placing a field (Username) where a program (browser) can't find it you are not supplying that program (browser) with the data it needs to produce the desired affect.

test

avatar Webdongle
Webdongle - comment - 30 Nov 2017

install screen 05

avatar infograf768
infograf768 - comment - 30 Nov 2017

@mbabker @dgt41 @brianteeman
I agree that it is a Firefox issue and not an issue in other browsers.

Now, if we can solve it for Firefox (as we did in 3.x) without creating issue for other browsers, why not do it? I just tested this PR in Safari and it saves username and password fine.

safari

I am sure you do not really want an important part of the user base to get into trouble just because the fields order is wrong for Firefox.

avatar infograf768
infograf768 - comment - 1 Dec 2017

As a reminder, don't forget Joomla adapts itself to specific browsers like IE concerning CSS and JS.
Also for our queries where we adapt to postgresql.

Therefore it is not unreasonable to adapt to Firefox.

Thanks for your attention

avatar infograf768 infograf768 - change - 1 Dec 2017
Title
[4.0] Correcting email field considered as a username field
[4.0] Firefox: Correcting email field considered as a username field
avatar infograf768 infograf768 - edited - 1 Dec 2017
avatar mbabker
mbabker - comment - 1 Dec 2017

To be clear, at no point have I suggested this not be merged now that the exact workflow and logic have been explained. But, it also needs to be clear that this is not the result of a bug in Joomla, no matter what some people want to believe. As stated and demonstrated, this is the result of a logic bug in the browser making an inference that causes an undesired workflow.

avatar infograf768
infograf768 - comment - 1 Dec 2017

totally agree with you, @mbabker

I guess our former Plt remarked that issue, and this is the reason why it is coded this way in 3.x and before.
I propose to merge this and see if we can find a solution later for the autofill stuff in these specific pages without a new <input as done in 3.x

avatar infograf768
infograf768 - comment - 2 Dec 2017

@wilsonge
Now that everyone understands the issue, could you please merge this?

avatar wilsonge wilsonge - change - 2 Dec 2017
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-12-02 13:50:38
Closed_By wilsonge
avatar wilsonge wilsonge - close - 2 Dec 2017
avatar wilsonge wilsonge - merge - 2 Dec 2017
avatar wilsonge
wilsonge - comment - 2 Dec 2017

Yup :) sorry for not replying on here earlier - it's been a very long week in the office!

avatar Webdongle
Webdongle - comment - 2 Dec 2017

@wilsonge

... it's been a very long week in the office!

Yep - been a long 10 days on here as well 😄 . Thanks

And thanks @infograf768 for sticking with me on this one and creating the patch.

Add a Comment

Login with GitHub to post a comment