Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Installation: email field is not a password field #18863

Closed
wants to merge 2 commits into from

Conversation

Webdongle
Copy link
Contributor

@Webdongle Webdongle commented Nov 25, 2017

Pull Request for Issue #18793

Summary of Changes

  1. Added dummy field in installation/tmpl/setup/default.php to prevent email field autofill with password in Firefox browser.
  2. Defined the field in installation/forms/setup.xml
  3. Moved the adnin field in installation/forms/setup.xml to match the order in the default.php

Testing Instructions

Download and unzip the Joomla 4 zip
Replace the installation/tmpl/setup/default.php to prevent email field autofill with password and installation/forms/setup.xml files
Run the install in Firefox browser

Expected result

Clicking in the email field (page 2 of the install) displays a list of emails not passwords

Actual result

Installing without the PR
Clicking in the email field (page 2 of the install) displays a list Passwords

Installing with the installation/tmpl/setup/default.php to prevent email field autofill with password and installation/forms/setup.xml files
Clicking in the email field (page 2 of the install) displays a list of emails not passwords

Documentation Changes Required

None

@ghost
Copy link

ghost commented Nov 25, 2017

@Webdongle which Issue-Number please?

@@ -30,12 +39,9 @@
/>

<field
name="admin_user"
name="dummy_input"
type="text"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you make the type hidden?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont know ... try it and see if you like but this way works.

@Webdongle
Copy link
Contributor Author

What are the failed tests > Have I done something wrong ?

@joomla-cms-bot joomla-cms-bot changed the title 4.0 dev [4.0] Installation: email field is not a password field Nov 25, 2017
@brianteeman
Copy link
Contributor

@Webdongle The drone errors are unrelated to your code. dont worry about them

@brianteeman
Copy link
Contributor

I will repeat my comments from the original issue. I do not see the problem that you are trying to solve. It is a feature of firefox that if you have a saved user in the browser then it will suggest the email that you saved for that user in the email address field

@Webdongle
Copy link
Contributor Author

@brianteeman

It is a feature of firefox that if you have a saved user in the browser then it will suggest the email that you saved for that user in the email address field

But when installing ... on the second page the email field does not offer email addresses it offers passwords.

email field offers passwords

install 01

install 01

@brianteeman
Copy link
Contributor

its not offering a password. it is offering the email that has been stored for that username in the browser.

@Webdongle
Copy link
Contributor Author

@brianteeman

its not offering a password. it is offering the email that has been stored for that username in the browser.

A KEY and the word admin is PASSWORD not email addresses
install 01b

@brianteeman
Copy link
Contributor

sorry but you are wrong. doesnt matter how many times you repeat it or how many pictures of glasses you draw. you are misunderstanding firefox.

@mbabker
Copy link
Contributor

mbabker commented Nov 25, 2017

OK, stupid question time. Have you actually clicked on one of the values that Firefox is offering in its autocomplete for that field? If so, what is the value it is giving (i.e. is it an email address or is it a password)? The only change I see here is the deliberate breaking of a browser feature because of one person's misunderstanding of the feature.

@brianteeman
Copy link
Contributor

Autofill has been completely rewritten in firefox quantum
This is an early report on it https://www.ghacks.net/2017/05/24/firefoxs-new-form-autofill-is-awesome/

@Webdongle
Copy link
Contributor Author

It is a username then the password field is auto filled to the value of the username

@brianteeman
Copy link
Contributor

@mbabker yes i clicked on it - i am not silly - and that is how i know it is giving me the email address as expected and as i have repeatedly stated

@dgrammatiko
Copy link
Contributor

Same functionality exists in safari, I can confirm what @brianteeman is saying, also that was the reason to say we should close the issue in the first place

@brianteeman
Copy link
Contributor

it is not rocket science guys.
joomla is working as intended
firefox is working as intended
PEBKAC

I would close this but as I am the main objector to this stupidity I will be politic and leave it to someone else

@Webdongle
Copy link
Contributor Author

Watch the email field it adds the user name that is stored in Firefox
With admin selected it inserts 'admin' and password field has 8 characters.
When another is selected it inserts 'another' and password field now has 9 characters
install 02

@mbabker
Copy link
Contributor

mbabker commented Nov 25, 2017

And there is the proof that it works as intended. You are selecting a value from a known list of accounts in Firefox's cache and Firefox is filling the password field based on your selection from another field. Looks like it's working to me. Closed.

@mbabker mbabker closed this Nov 25, 2017
@brianteeman
Copy link
Contributor

Thats because that is what you have stored in firefox!!!!

@Webdongle
Copy link
Contributor Author

Webdongle commented Nov 25, 2017

@mbabker

And there is the proof that it works as intended.

No that proves the email autofill does not work as intended. The email field presents users not emails

Please look again 'admin' and 'another' are usernames not email address
install 03

@mbabker
Copy link
Contributor

mbabker commented Nov 25, 2017

Sorry, we are not accepting pull requests which disable browser features. If you feel there is a bug in how the browser feature works, submit a bug report to the appropriate vendor.

@brianteeman
Copy link
Contributor

brianteeman commented Nov 25, 2017

i will say this for the last time
the problem is with YOUR browsers saved values.

It is nothing to do with Joomla or Firefox there is not bug with either of them.

if you had an email address stored then it would look like this

password

@Webdongle
Copy link
Contributor Author

@brianteeman
If you enter the email manually then you can select the password for a user in the password field. But if you click the email field then user/pass are presented not emails

@mbabker
It is perhaps a browser bug or perhaps its' the way Joomla renders the fields because it doesn't happen if the fields are text not php calls.

This is how it should work (and works this way if the php in the default.php is replaced with normal text or with the proposed patch)
install 06

@brianteeman
Copy link
Contributor

i have used up my limited spare time to deal with this

@mbabker
Copy link
Contributor

mbabker commented Nov 25, 2017

Joomla renders standard HTML. The browser doesn't care if the HTML is rendered by PHP, JavaScript, Java, Python, Perl, or any other programming language. So no, browsers do not behave differently because of the tooling used to build the resulting HTML.

Your patch explicitly breaks a browser feature. Therefore it is invalid. The fact you need a dummy input to get the result you want points to a browser bug, not a Joomla bug, because Joomla doesn't control the browser handling. The absolute most that we might be able to do is switch the order on the username and email fields, but such a decision has absolutely nothing to do with browser auto fill logic.

@Webdongle
Copy link
Contributor Author

And you still cant see what the problem is. Never mind, I've wasted enough time trying to explain why the email field should display emails not the key with a user/pass.

It could be a browser bug or it could be a joomla bug ... but when <?php echo $this->form->getLabel('admin_email'); ?>is replaced with the standard for notation then it doesn't happen. So that suggests that the browser is recognising the email field as a text field because of the way Joomla renders it.

If it is not fixed then there will be a lot of complaints in the forum. And it will be off putting for new/potential users.

@brianteeman
Copy link
Contributor

please learn how to use your browser

ALSO your patch is no good as it is fails both standard html standards (missing closing div) and accessibility standards (display none )

@mbabker
Copy link
Contributor

mbabker commented Nov 25, 2017

Define standard notation. Are you saying that the HTML which is rendered on the page has been used to avoid the PHP method call or have you crafted your own markup to prove a point?

The browser doesn't know the page was rendered with PHP. The browser only knows the HTML it was given. I'm not entertaining an argument that equates to "the browser handles hand crafted HTML differently from dynamically generated HTML" because that is a flat out lie.

@Webdongle
Copy link
Contributor Author

@mbabker
OK. Just one question ... why does the install page have the email and password fields together when the site login is user/password not email/password ?

because that is a flat out lie.

I make mistakes but I do not lie. Please do not insult me like that it is not keeping with the spirit cooperation.

@brianteeman
Copy link
Contributor

because we need the email as part of the signup but we do not need the email as part of the login

@Webdongle
Copy link
Contributor Author

But the username is needed as part of the sign in and login. With 3.x the signup was
email
user
password
confirm password

but 4.0 it has changed to
user
email
password

What was the logic of changing the order of the fields ?

@brianteeman
Copy link
Contributor

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

@joomla joomla locked and limited conversation to collaborators Nov 25, 2017
@infograf768
Copy link
Member

Sorry folks, but as I have demonstrated here
#18793 (comment)
we do have an issue and the code proposal I made does work to solve it.

@infograf768
Copy link
Member

The best proof of this is installing 3.x with Firefox Quantum where the email field comes first.
I typed i in the field and I get

autofillemail

@infograf768
Copy link
Member

Now let's place the email field BEFORE the Username in J40
autofillemailj40

@infograf768
Copy link
Member

infograf768 commented Nov 28, 2017

Therefore, I suggest to use the other solution and indeed move the email field before the username in setup.php (Still FF Quantum...)

			<div class="j-install-step-form">
				<div class="form-group">
					<?php echo $this->form->getLabel('admin_email'); ?>
					<?php echo $this->form->getInput('admin_email'); ?>
				</div>
				<div class="form-group">
					<?php echo $this->form->getLabel('admin_user'); ?>
					<?php echo $this->form->getInput('admin_user'); ?>
				</div>
				<div class="form-group">
					<?php echo $this->form->getLabel('admin_password'); ?>
					<?php echo $this->form->getInput('admin_password'); ?>
				</div>
				<div class="form-group">
					<button class="btn btn-primary btn-block" id="step2"><?php echo JText::_('INSTL_CONNECT_DB'); ?> <span class="fa fa-chevron-right" aria-hidden="true"></span></button>
				</div>
			</div>

Thank you for your attention

@infograf768
Copy link
Member

Will make PR.

@infograf768
Copy link
Member

Please see #18911

@Webdongle Webdongle deleted the 4.0-dev branch December 2, 2017 14:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants