Skip to content

Conversation

@bahl24
Copy link
Contributor

@bahl24 bahl24 commented Feb 15, 2019

Signed-off-by: Nitish Bahl [email protected]

Pull Request for Issue - Go to user menu at top right corner -> Edit account -> see that show pass icon alongside password doesn't work while that alongside confirm pass works.

Summary of Changes

Autocomplete is already false thus extra condition is not required

Expected result

Show pass field works like that in com_users

Actual result

Nothing happens when icon is clicked

Documentation Changes Required

No

@brianteeman
Copy link
Contributor

While this does work it is removing code that at least according to the comment is there to prevent autocomplete

@bahl24
Copy link
Contributor Author

bahl24 commented Feb 16, 2019

@brianteeman You can verify by debugging that autocomplete is already false, thus the commented code is not required. Plus, no such code is there in com_users/edit which serves the same purpose and thus has no extra security feature.

@brianteeman
Copy link
Contributor

So why was it there?

Sorry but when I see a comment like that then we need to investigate why it was put there and if it is no longer required.. You can';t just delete it

@infograf768
Copy link
Member

although it was slightly modified (password instead of text), that code was there in 3.x to cope with Firefox.
see
#7094

as it is quite old stuff, we should test the code we have now in 4.0 on Firefox.

@dgrammatiko
Copy link
Contributor

@brianteeman @infograf768 @bahl24 thisis definetely not the way to fix this!!!

The correct fix is to change the autocomplete to new-password in here:

The explanation is here: https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion#The_autocomplete_attribute_and_login_fields

or if you're too lazy this part:
screenshot 2019-02-16 at 20 26 42

@bahl24
Copy link
Contributor Author

bahl24 commented Feb 16, 2019

@dgrammatiko As mentioned in the last line, this has not been implemented yet.
See https://stackoverflow.com/questions/17719174/autocomplete-off-is-not-working-when-the-input-type-is-password-and-make-the.
I think we have same problem in Users->Manage->edit user. Adding code to disabling autocomplete should be added here as well. Should I do it?

@dgrammatiko
Copy link
Contributor

@bahl24 works for me 🤷‍♂️

@bahl24
Copy link
Contributor Author

bahl24 commented Feb 16, 2019

@bahl24 works for me man_shrugging

Ok it might be implemented now, let me change autocomplete to new pass, which eliminates the need of <?php // Disables autocomplete ?> <input type="password" style="display:none">

Signed-off-by: Nitish Bahl <[email protected]>
@brianteeman
Copy link
Contributor

hey I dont even know if it is desirable to prevent autofill. I was just pointing out that we cant delete code without full investigation of why it was there. Otherwise we end up with bug reports on a new release

@bahl24
Copy link
Contributor Author

bahl24 commented Feb 17, 2019

@brianteeman As @infograf768 pointed out kindly see pr #7094 in which the following lines of code was added to prevent auto-filling of the forms. But since then many changes have been introduced in firefox such as autocomplete=new-password to get rid of the problem.
Also you can see in Users->Manage->edit user, which provides the same funtionality, that no such code exists in edit.php file

@bahl24
Copy link
Contributor Author

bahl24 commented Feb 17, 2019

@brianteeman
screenshot from 2019-02-17 23-43-33
This is the page I am talking about


No such code exists & show pass icon works properly

@infograf768
Copy link
Member

infograf768 commented Feb 18, 2019

@dgrammatiko
changing to

		<field
			name="password"
			type="password"
			label="JGLOBAL_PASSWORD"
			autocomplete="new-password"
			class="validate-password-strength"
			filter="raw"
			validate="password"
			strengthmeter="true"
			force="on"
			size="30"
		/>

		<field 
			name="password2" 
			type="password"
			label="COM_USERS_USER_FIELD_PASSWORD2_LABEL"
			autocomplete="new-password"
			class="validate-passwordExtra"
			filter="raw"
			message="COM_USERS_USER_FIELD_PASSWORD1_MESSAGE"
			size="30"
			validate="equals"
			field="password"
		/>

just does not work for Firefox in 4.0. for the password fields
screen shot 2019-02-18 at 15 34 54

The Password field itself is always filled when editing a user (testing as super Admin here), thus forcing to enter again the code

						<div class="controls">
							<?php if ($field->fieldname == 'password') : ?>
								<?php // Disables autocomplete ?> <input type="password" style="display:none">
							<?php endif; ?>
							<?php echo $field->input; ?>
						</div>

Therefore, the feature MAY work in broswers in general, BUT not for Firefox

Any real-world working solution is welcome.

@bahl24
Copy link
Contributor Author

bahl24 commented Feb 18, 2019

@infograf768 But such code does not exist in https://github.com/joomla/joomla-cms/blob/23d5fb7edc3046f598a8318e80777f8a1ad75985/administrator/components/com_users/tmpl/user/edit.php#L41though both com_user and com_admin give same functionality of changing passwords

@infograf768
Copy link
Member

I am here talking about the changes I had to do to NOT get the password field already filled in


as, when it is already filled, I have to fill again the Confirm Password field each time I have to modify some details of the user, which is not expected.
the code above concerns password and not password2

@dgrammatiko
Copy link
Contributor

@infograf768 what version of FF are you using?

@infograf768
Copy link
Member

@infograf768 what version of FF are you using?

last one: 65.0.1 Macintosh Clean branch install

@dgrammatiko
Copy link
Contributor

@infograf768 the added HTML markup is not the way forward here. This is: https://gist.github.com/jonathantneal/d462fc2bf761a10c9fca60eb634f6977

@infograf768
Copy link
Member

@dgrammatiko
I suppose the code you link to would solve the issue. I don’t have the slightest idea on using it to propose a patch.

@ghost ghost added the J4 Issue label Apr 5, 2019
@ghost ghost removed the J4 Issue label Apr 13, 2019
@ghost ghost changed the title [4.0]show pass not working in edit account [4.0] show pass not working in edit account Apr 19, 2019
@ahghatol
Copy link

It works for me without applying a patch.

@nidhi1709
Copy link

I have tested this item ✅ successfully on f46d0a1

It worked without applying patch.

Also check on firefox version 69.0.2 and it worked correct.


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

@brianteeman
Copy link
Contributor

If it worked without the patch then it cant be a successsful test of the patch

@deepa-g
Copy link

deepa-g commented Oct 19, 2019

I have tested this item ✅ successfully on f46d0a1


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

@idefax
Copy link

idefax commented Oct 19, 2019

No need to apply patch, it works without it.


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

@Quy Quy closed this Nov 4, 2019
@Quy Quy added the J4 Issue label Nov 4, 2019
@Quy
Copy link
Contributor

Quy commented Nov 4, 2019

Closing as the proposed change is in the codebase and the proposed deletion is no longer there.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants