Page MenuHomePhabricator

Cloner fields are unintentionally removed in create/edit pages
Closed, ResolvedPublic

Description

This only affects users without JavaScript.

This was largely solved in T270200, but still affects some browsers. From T270200#6740254:

This bug still affects Safari 14 and IE11, but is fixed on Firefox, Edge and Chrome.

The problem applies to the Question and Option fields at the bottom of the form. (The Admin field was fixed in T270634.)

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald Transcript

From what I can tell (testing on Safari 13), because that hidden submit button is, well, hidden, it's not being used as the submit when the form is submitted via the enter key. Instead, Safari considers the submit from the cloner as the first submit and therefore triggers that event instead. If I manually make the submit button visible again, Safari recognizes it and does not submit w/the delete cloner value. Considering that we're supporting no-JS, I don't think we can prefer buttons over submit w/value so off the top of my head, I think we have to keep the submit button at the beginning of the form but also make it visible.

The fastest way would be to simply do that. @Prtksxna do you think that's fine? Otherwise I can think of a few visibility hacks we can try out if we're okay with that.

Niharika moved this task from Untriaged to Cards ready to be discussed on the Anti-Harassment board.

@STran could you share a screenshot of the page? Would it be possible to move the button to the bottom of the form?

Sure thing:

and I'm asking if something like this is possible:

Would it be possible to move the button to the bottom of the form?

It's more like we added in the button at the top in order to solve the original problem. So when you press 'enter', it submits the form by essentially triggering a click on the first submit in the form DOM. The problem is that we support no-js by passing along additional values w/certain submits. We hack around this by adding in a hidden submit at the very top of the form so that it's considered the first submit instead of the cloner element submit (which is what was causing the array of "randomly disappearing inputs" symptoms). Unfortunately, because it's hidden, it looks like Safari and IE are ignoring it and instead using the first visible submit.

I think we have to keep the submit button at the top around because we want to continue supporting no-js. In that case, we can either 1. make it completely visible like in these screenshots or 2. we can try some visibility hacks. I can think of a few things to try for option 2 but option 1 would be an easier solution.

Thanks for the screenshots and explanation, @STran. We aren't using the pattern of having submit button both on the top and bottom in MediaWiki. We could make it work but it might require more design and subsequently development work. I'd suggest we go with option 2.

Change 657057 had a related patch set uploaded (by STran; owner: STran):
[mediawiki/extensions/SecurePoll@master] Visually hide the submit button only

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

@STran I notice this bug also affects the "Exclude users in these groups" and "Include users in these groups, regardless of edits or other groups" inputs in the Special:SecurePoll/votereligibility/$id form.

Will that be addressed in this change or should I open a new ticket?

Oh good catch ๐ŸŽ‰ I will fix it in this patch. I also grepped around to see if there were any other pages (any page w/a cloner element will have this problem) and it seems like it's just these two.

Tested this and it works on Safari 14 but not IE 11.

I'd be happy to go ahead with this patch and file another task for IE11. @Niharika what do you think the priority should be for fixing this for IE11? The issue can be worked around by not pressing enter at the wrong time, or clicking the add/remove buttons (admins have been doing that on all browsers for a while!).

@dom_walden The patchdemo linked to above is old - there's an up-to-date one at https://patchdemo.wmflabs.org/wikis/e6e8f848a7b252a73dc7d081efc218fa/w/index.php/Main_Page

@Niharika Please ignore my question, @STran found a solution for both browsers!

Change 657057 merged by jenkins-bot:
[mediawiki/extensions/SecurePoll@master] Visually hide the submit button only

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

I can no longer reproduce the bug on IE11, Safari 14 and Safari 8.

Now, if you press enter in another field, the cursor is automatically taken to the cloner field (on IE11 and Safari 14, even if JS disabled) or the form submits but returns a validation error (Safari 8).

I was able to use those browsers to successfully submit the /votereligibility form with the two cloner fields filled.

On /create, we no longer use the cloner fields (we have changed to using the UsersMultiSelect widget like we do elsewhere).

Test Environment Patch Demo MediaWiki 1.36.0-alpha (2f433de), SecurePoll 2.0.0 (02a680d) 11:14, 21 January 2021.