Page MenuHomePhabricator

Special:Preferences asks for form data save confirmation beforeunload even if nothing was changed
Closed, ResolvedPublic

Description

  1. Go to Special:Preferences (with JavaScript enabled) and wait until it completely loads.
  2. Do not change any form data.
  3. Navigate to another link (for eg. click on the Watchlist link in personal tools)
  • Actual: Popup appears asking for confirmation "This page is asking you to confirm that you want to leave - data you have entered may not be saved.".
  • Expected: It shouldn't appear if nothing was changed.

Browser/OS: Firefox 43, Ubuntu

Seems to have been caused by https://gerrit.wikimedia.org/r/#/c/255652/

Event Timeline

Glaisher created this task.Dec 31 2015, 3:57 PM
Glaisher updated the task description. (Show Details)
Glaisher raised the priority of this task from to Needs Triage.
Glaisher added a subscriber: Glaisher.
Restricted Application added subscribers: StudiesWorld, Aklapper. · View Herald TranscriptDec 31 2015, 3:57 PM
Sn1per added a subscriber: Sn1per.Dec 31 2015, 4:05 PM
Sn1per updated the task description. (Show Details)Dec 31 2015, 4:18 PM
Sn1per set Security to None.
Glaisher updated the task description. (Show Details)Dec 31 2015, 4:20 PM

I can't seem to reproduce this with Chrome 47 on Windows, Firefox 43 on Windows, or Firefox 42 on Fedora. Could someone with Ubuntu confirm?

TheDJ added a subscriber: TheDJ.Dec 31 2015, 10:35 PM

At which wiki was this ? It might be that form contribution by an extension might influence something.

I can't reproduce either. I tried Firefox 43 on Ubuntu 15 (inside a VM), locally and on Beta labs.

It was on my local test wiki. After some further testing, it looks like the Translate extension's translate-editlangs pref which uses a weird HTMLFormField is doing this. HTMLJsSelectToInputField apparently doesn't work at all when JS is disabled (and that's pretty bad in itself considering that the element is there always).

TheDJ added a comment.Jan 1 2016, 8:47 PM

translate-editlangs is used as a control for another field and doesn't have an actual 'state' (reflected by the fact that there is no initial selected option in this case either). The script isn't expecting this and thus also cannot handle it.

Anomie added a subscriber: Anomie.Jan 16 2016, 5:19 PM

I can reproduce this on enwiki. In this case, setting some breakpoints in the JavaScript tells me that it's deciding #mw-input-wptimecorrection-other was changed. To reproduce this instance of the problem, you probably need the time zone dropdown set to something other than "other".

I get this on enwiki, cywiki, dewiki, frwiki, commons, meta and probably all the others. On all Wikimedia projects, my time zone is always set to "Use wiki default (UTC)", "Use wiki default (Europe/Berlin)", "Use wiki default (Europe/Paris)" or similar, if available - which is what I set right from the start (May 2009 in the case of enwiki).

Yep, can confirm the time zone issue. I'll see if I can fix it.

Ok, so the issue is that the Other box is set client-side, so the "changed prefs" check thinks that something has changed.

Change 264549 had a related patch set uploaded (by Sn1per):
Set defaultValue property on tz box in Special:Preferences to fix change check

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

Change 264555 had a related patch set uploaded (by Sn1per):
In Special:Preferences, update defaultSelected for options in language selector

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

TheDJ added a comment.Jan 17 2016, 3:18 PM

@Sn1per, hmmm, i'm not a fan of solving this per preference. How about we just ignore input's without the value/selected/checked attribute ?

Change 264603 had a related patch set uploaded (by TheDJ):
[Do not merge] Ignore select state if there was no default

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

TheDJ added a comment.Jan 17 2016, 5:30 PM

For the timezone, it's the best way. For translate, I'm not sure. I uploaded an alternate patch, that might be more sensible for that case, but I haven't tested that yet.

Change 264549 merged by jenkins-bot:
Set defaultValue property on tz box in Special:Preferences to fix change check

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

Change 264555 abandoned by Sn1per:
In Special:Preferences, update defaultSelected for options in language selector

Reason:
I think I1cf27267e60f would be a better idea.

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

Change 264609 had a related patch set uploaded (by Sn1per):
Revert "Set defaultValue property on tz box in Special:Preferences to fix change check"

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

Florian added a subscriber: Florian.

Can somebody summarize current state? As far as I can see, there are two problematic prefs, one being the core timezone one (affecting "global" wikis like English Wikipedia the most, but not "local" ones like, say, Polish Wikipedia where most editors live in the default time zone of the wiki), and the other being the Translate extension's preferred languages (affecting any wiki where Translate is installed). Which do we have patches for? :D

A more general solution would be for the beforeunload handler to only check form fields with a 'name' attribute (the others are not submitted with the form anyway). It looks like this would work for Translate (it'd just need to drop the unnecessary 'name' from the dropdown). For the core timezone, we might need to mirror the selected value to a hidden input with the 'name'? – I'm not sure what this even does… This would also work with any OOjs UI widgets we might use for preferences in the future.

A patch has been merged to cover the timezone issue (https://gerrit.wikimedia.org/r/#/c/264549/), but it is flawed and should probably be reverted and replaced with @TheDJ's superior solution which only checks form fields that have default values: https://gerrit.wikimedia.org/r/#/c/264603/

Jay8g added a subscriber: Jay8g.Jan 18 2016, 6:50 AM
Sn1per triaged this task as High priority.Jan 18 2016, 1:40 PM
Nikerabbit added a subscriber: Nikerabbit.

I removed MediaWiki-extensions-Translate as per my understanding no changes were needed in it.

TheDJ added a comment.EditedJan 27 2016, 9:24 PM

I actually think that my suggested patch is more foolproof than what Matmarex was proposing for this specific problem. Possibly both strategies could even be combined and done in series however for 'maximum' coverage of possible edge cases (Update the patch to do this).

Looks like solved or not?

I am still having the issue on Wikidata for example.

aude added a subscriber: aude.Feb 24 2016, 9:20 AM

I have this issue also. (with chromium)

what the...
It seems that the first option in the languages dropdown has the selected state, but that's weird, since it's aa - Afar, and not en - English, which is the actual selected option visually....

Grah.

I am also seeing this on translatewiki.net. So is Translate extension's preferences among the causes?

TheDJ added a comment.Feb 24 2016, 2:25 PM

It seems so. I guess we will have to look into it a bit deeper.

I'm seeing it fixed on Wikipedia for chrome, mac.

Jay8g added a comment.Feb 25 2016, 6:32 AM

For me (Firefox on mac) it's working on enWP but not on MediaWiki.

Dvorapa removed a subscriber: Dvorapa.Feb 25 2016, 9:34 AM
TheDJ added a comment.Feb 25 2016, 9:57 PM

Right, my earlier proposed patch would solve this. I'll try to poke someone for expediting the merge and deploy

For me (Firefox on mac) it's working on enWP but not on MediaWiki.

There's a patch that fixes the timezone box in core which was causing the issue, but the issue is still caused by Extension:Translate, which I believe is on the mediawiki wiki etc. As previously mentioned, a patch is in the works to fix this universally, after which the fix for the timezone box is probably extraneous.

Change 273377 had a related patch set uploaded (by TheDJ):
Do not submit value of JS control mw-language-selector

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

TheDJ added a comment.EditedFeb 25 2016, 10:45 PM

Right, so @matmarex was right here I think, so incorporated his suggestion and made a patch to Translate extension that as a combination should fix this.

Also, I'm on vacation starting tomorrow, so I can't shepherd this through a SWAT or anything. I'm pretty sure @Lydia_Pintscher can sweet talk someone else into doing that however ;)

Change 273377 merged by jenkins-bot:
Do not submit value of JS control mw-language-selector

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

Elitre added a subscriber: Elitre.Mar 7 2016, 9:03 AM

Change 264603 merged by jenkins-bot:
Ignore name-less inputs on preference page confirmCloseWindow check

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

Looks like that's all?

Change 275745 had a related patch set uploaded (by Krinkle):
Ignore name-less inputs on preference page confirmCloseWindow check

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

Change 275745 merged by jenkins-bot:
Ignore name-less inputs on preference page confirmCloseWindow check

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

Krinkle closed this task as Resolved.
Krinkle claimed this task.

Change 264609 abandoned by Sn1per:
Revert "Set defaultValue property on tz box in Special:Preferences to fix change check"

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