Page MenuHomePhabricator

Captcha positioning is inconsistent between JavaScript and non-JavaScript users
Closed, ResolvedPublic

Description

The HTML rendered for non-JavaScript and JavaScript users is different. it's okay to use enhancements but they should be at least positioned correctly.

The way I tend to explain this is a problem is using the following scenario. You ring your grandfather and try to help him register for an account. You tell him there is a captcha he needs to fill above the create account button as you see this:

.

He might have a bad connection/browser though that we don't load JavaScript on so he sees this:

Let's make sure the initial HTML appears in the correct place.
This also blocks a mobile web card T74910

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: Jdlrobson.
Sumit added a subscriber: Sumit.Dec 25 2014, 6:50 PM

The js styles the fancy captcha and then moves it down. I think we can retain the styling in case js is enabled, but do not allow re-positioning of captcha so that the location of captcha always remains at the top, whatsoever be the case?

Sumit added a comment.EditedDec 25 2014, 7:03 PM

or else we can display the captcha just before submit,as it appears when js enabled, by default...I'd like to work on this, but need to know where should I place the captcha, top or bottom?

I would say the latter. Instead of moving it down with JavaScript, the captcha should not need to be repositioned.

The preferred position is below the submit form as it appears when js enabled.

Thanks @Sumit! Please do claim it and I will happily review! :)

Aklapper triaged this task as Low priority.Dec 28 2014, 11:08 PM
Sumit claimed this task.Dec 31 2014, 8:26 PM

@Jdlrobson, I'll soon submit a patch, but I need a little advice on one thing - the confirmedit extension inserts the captcha into the header of the signup form in Usercreate.php. I could've simply moved the header down, but then I can do that safely only if no other extension or info is being put into the header. So, is there a way to know if the header is being used for something else too?

Don't move the header down as it may be used by other things. Instead the extension should instead insert the captcha into a more appropriate location. Might require a core change to support htis?

Sumit added a comment.Jan 2 2015, 11:26 AM

The captcha could be inserted into a separate element of its own identified by some class, and then in all the five pages where the captcha is meant to appear, we can add that element, just before the submit button. Would that be right?

Sounds good to me. Submit a patch and we can work out any issues during code review! This may require tinkering with core to provide some kind of hook to support this. I'm not too familiar with this code right now so I can't say for sure.

@Sumit: Do you need some help, or have you questions?

The main problem is, that the UserCreate form doesn't allow to inject html except to the header. You can add special fields, but only checkboxes or other inputs:
https://github.com/wikimedia/mediawiki/blob/master/includes/templates/Usercreate.php#L204

So i suggest, that you copy the extrafields strategy from UserLogin template:
https://github.com/wikimedia/mediawiki/blob/master/includes/templates/Userlogin.php#L123

so ConfirmEdit can add the form html to the extrafields parameter :)

Sumit added a comment.EditedJan 13 2015, 9:19 PM

I've added a patch for this bug, don't understand why gerrit fails to list it here, its at https://gerrit.wikimedia.org/r/#/c/184775/
@Florian I thought of adding a captcha placeholder to both the forms, after which we can set it from ConfirmEdit. However if thats the wrong way, I'll go ahead with your suggestion.
I guess I also need to add a separate patchset, modifying the functionality of ConfirmEdit extension?

Change 184775 had a related patch set uploaded (by Florianschmidtwelzow):
Templates:Captcha placeholder added to Userlogin,Usercreate

https://gerrit.wikimedia.org/r/184775

Patch-For-Review

I guess I also need to add a separate patchset, modifying the functionality of ConfirmEdit extension?

Correct :) Thanks for your work on this!

Change 184912 had a related patch set uploaded (by Sumit):
Captcha.php: modified to add captcha to 'extrafields' in Userlogin and Usercreate

https://gerrit.wikimedia.org/r/184912

Patch-For-Review

Hey @Sumit I've left you some feedback on the patch. Once you've folded in my changes I would say this is good enough state to merge.

Change 184775 merged by jenkins-bot:
Templates:Captcha position modified,extend functionality added to QuickTemplate

https://gerrit.wikimedia.org/r/184775

Change 184912 merged by jenkins-bot:
Captcha.php: modified to add captcha to 'extrafields' in Userlogin and Usercreate

https://gerrit.wikimedia.org/r/184912

Jdlrobson closed this task as Resolved.Jan 20 2015, 11:08 PM

I noticed 66f62346ae30470412f45c99ece51e3cb70c5152, deployed on Wikipedias on 2015-02-11.

Change 184775 merged by jenkins-bot:
Templates:Captcha position modified,extend functionality added to QuickTemplate
https://gerrit.wikimedia.org/r/184775

This commit message is very hard to read for me and the release notes only say "modified", can someone explain what the change actually did?

@Nemo_bis: The captcha was added as a header field to the user create/login form and moved with JavaScript to the bottom (as the last field before the submit button). With this change, the Form get's a new field, "extrafields", for extensions like ConfirmEdit, to add fields before the submit button directly, so there is no need to move the CAPTCHA with JavaScript anymore (and JS and No-JS experience is the same :)).

Thanks for the explanation.