585591 - Password manager detects an invisible input with display:none as the username field (affects vBulletin 4)

Status: NEW

(Toolkit :: Password Manager, defect, P3)

Description

[Oleg]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b4pre) Gecko/20100809 Minefield/4.0b4pre
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b4pre) Gecko/20100809 Minefield/4.0b4pre

Password manager can't restore Login/Password for new vBulletin version.

Reproducible: Always

Steps to Reproduce:
1. Login to forum.
2. Click "Remember" for forum.
3. LogOut from forum.
Actual Results:  
Unable to login with saved Authorization data.

Comment 1

[Oleg]

Same problem on http://www.mv-detox.com/forum.php

Comment 2

[Oleg]

And same problem on http://defendium.info/forum.php

Comment 3

[Oleg]

Attachment: password manager

Comment 4

[Oleg]

http://filesharingtalk.com/ same

Comment 5

[Alexander L. Slovesnik]

It looks like vBulletin bug http://tracker.vbulletin.com/browse/VBIV-6360 which has been fixed in vBulletin 4.0.8

Comment 7

[Matthew N. [:MattN]]

(In reply to Alexander L. Slovesnik from comment #5)
> It looks like vBulletin bug http://tracker.vbulletin.com/browse/VBIV-6360
> which has been fixed in vBulletin 4.0.8

I can't see that tracker bug and I'm also seeing it affecting http://forums.vwvortex.com which claims to be running version 4.2.1.

The problem is that there is a hidden <input type="text" value="Password" style="display:none"> at submision time that vBulletin uses to display the "Password" placeholder text before the password field is focused. Upon focusing the placeholder field, it is swapped with the real password field.

This should probably be fixed in vBulletin as they should use the placeholder attribute on <input> in supported browsers to get a behaviour that is easier for browsers to understand. It may already be fixed in vBulletin 5 as most affected sites seems to be running version 4 AFAICT. I will attach a reduced testcase.

Comment 8

[Matthew N. [:MattN]]

Attachment: Reduced test case

Comment 9

[Matthew N. [:MattN]]

vBulletin could have also switched the hidden filed to type="hidden" to prevent this as type="hidden" is not recognized as a username field type[1]. We could make _isUsernameFieldType from [1] smarter and ignore fields with display:none and maybe visibility:hidden at the risk of breaking other websites.

[1] https://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm?rev=e7d886615ad8#267

Comment 10

[Matthew N. [:MattN]]

Alexander, do you still have access to the tracker issue in comment 5 and if so, can you provide me with a copy of the information?

Comment 11

[Alexander L. Slovesnik]

Attachment: http://tracker.vbulletin.com/browse/VBIV-6360

(In reply to Matthew N. [:MattN] from comment #10)
> Alexander, do you still have access to the tracker issue in comment 5 and if
> so, can you provide me with a copy of the information?

See attachment.

Comment 12

[Matthew N. [:MattN]]

(In reply to Alexander L. Slovesnik from comment #11)
> Created attachment 8346427 [details]
> http://tracker.vbulletin.com/browse/VBIV-6360
> 
> (In reply to Matthew N. [:MattN] from comment #10)
> > Alexander, do you still have access to the tracker issue in comment 5 and if
> > so, can you provide me with a copy of the information?
> 
> See attachment.

Thank you very much.

I see that the bug is still open in their tracker and luckily some people have found workarounds such as re-ordering the password placeholder to be after the real password field.

If anyone knows whether this is fixed in vBulletin 5, please share so we can prioritize this bug accordingly.

Dolske, do you think that the proposal in comment 9 makes sense? 

An alternative is to modify the loop at [1] to use the first field before the password field as the username unless it's display:none (and similar) in which case we continue searching further to find the first non-display:none field to replace |usernameField|. We could even limit that to looking only one element further to be conservative.

Both proposals may break existing working sites IMO but it's hard to predict.

[1] https://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerContent.jsm?rev=e7d886615ad8#311

Comment 13

[[e-mail]]

Related thread in forum discussing the issue:

http://www.sitepoint.com/forums/showthread.php?871009-Why-put-input-hidden-field-in-display-none-style

Comment 14

[Justin Dolske [:Dolske]]

This is basically the opposite of the issue reported in bug 733217, no?

The vBulletin report (attachment 8346427 [details]) makes it sound like it's broken in a number of other browsers, probably for the same reason. Sounds like the proper resolution is for vBulletin to fix their code. Especially given that making this change could break password manager on other sites (ala 733217), and this doesn't seem to have been issue reported outside of vBulletin.

Additionally, if I'm reading the report correctly, this field is just being used to show an input with "Password" in it, as a clue for what to type there. The modern way to do this in HTML is with the |placeholder| attribute... <input type=password placeholder="Type yer password here!"> See bug 888806 where we fixed this for Bugzilla!

I'd suggest we WONTFIX this bug.

Comment 15

[[e-mail]]

The vwvortex webmaster has been notified of the issue.

Comment 16

[Matthew N. [:MattN]]

(In reply to Justin Dolske [:Dolske] from comment #14)
> I'd suggest we WONTFIX this bug.

I hesitated to do so since there are a decent number of vBulletin 4 sites in existence that are affected.

Can anyone confirm that this is fixed in a default installation of vBulletin 5 in all standard themes?

Comment 17

[m.kurz]

+1

Comment 18

[m.kurz]

I created a jsfiddle to reproduce this problem:
http://jsfiddle.net/nU8c3/show/

Comment 19

[m.kurz]

Similar Bug 997685

Comment 20

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:serg, could you have a look please?

For more information, please visit BugBot documentation.