Page MenuHomePhabricator

Skip tutorial does not set user preference in Chrome and Firefox
Closed, ResolvedPublic

Description

seen in Chrome but not in Firefox:

as a logged in user make sure that http://commons.wikimedia.beta.wmflabs.org/wiki/Special:Preferences#mw-prefsection-uploads shows the "Skip introductory licensing tutorial" preference is unchecked

On the UW page http://commons.wikimedia.beta.wmflabs.org/wiki/Special:UploadWizard

click the checkbox for "Skip this step in the future"

see message "You can re-enable this tutorial in your user preferences, under Upload Wizard → User interface."

click Next

go to http://commons.wikimedia.beta.wmflabs.org/wiki/Special:Preferences#mw-prefsection-uploads

"Skip introductory licensing tutorial" remains unchecked


Version: unspecified
Severity: normal

Details

Reference
bz70303

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:42 AM
bzimport set Reference to bz70303.
bzimport added a subscriber: Unknown Object (MLST).

Update: happens in Firefox also it seems but sometimes the setting will actually take permanentlty

I have reproduced this manually a number of times also.

neilk wrote:

Can't reproduce this.

It might be a race condition or speed issue.

I often see the issue on beta labs (especially with Chrome) when I do the first three steps very quickly:

On http://commons.wikimedia.beta.wmflabs.org/wiki/Special:UploadWizard:

  • check the box for "Skip this step in the future"
  • click "Next"
  • click Preferences in the upper right of the page

I was also able to reproduce it when I follow the steps quickly.But, if I wait for a while after checking the checkbox, then the checkbox remains unchecked in the preference section

neilk wrote:

It's true that in the code, we do not block on preference setting before going to the next step in the UploadWizard.

So if you're very quick and/or MediaWiki is very slow, and you unload the UploadWizard page by going to the Preference page, then yes, that preference won't be set.

We could block on preference setting, for the sake of testability, but then we need to keep the user waiting and figure out some kind of UI if preference setting is taking a long time or fails. This doesn't seem like a huge win to me.

If we can't guarantee that the preference gets set then we should delete the test for it.

Alternately, let's just not set the preference at all and simply change the user prompt from

"You can re-enable this tutorial in your user preferences, under Upload Wizard → User interface"

to

"You can stop seeing this tutorial by setting your user preferences, under Upload Wizard → User interface"

neilk wrote:

We're not going to drop a feature because we're having problems testing it.

I think your first suggestion is better; maybe this is not browser-testable. If we don't do anything in the UW interface itself to indicate success or failure, there's nothing to test.

I guess I will undo the whole thing I did yesterday.

neilk wrote:

Okay I removed the related automated test. I don't seem to have the authority to mark this a WONTFIX, but that's my recommendation.

I am still a bit concerned that users will see an instruction that may or may not actually function as stated, but it's not a big deal.

neilk wrote:

Okay yesterday I suggested we close this, but I think there is a way to fix it.

We can add a confirmCloseWindow() to the AJAX call. This is a feature in UploadWizard (well, actually MediaWiki now, it got generalized) that will ask the user to confirm if they want to navigate away. How does that sound?

I'm going to defer to Mark and Fabrice about the desired behavior of UW. My interest is in consistent behavior (whatever it may) and tests for that behavior.

neilk wrote:

Potential fix in Gerrit as described in comment 11 above.

https://gerrit.wikimedia.org/r/#/c/158249/

Using http://commons.wikimedia.beta.wmflabs.org/wiki/Special:UploadWizard?debug=true and clicking the checkbox, I get

Validation error against schema UploadWizardTutorialActions: Unknown schema: UploadWizardTutorialActions load.php?debug=true&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=20140907T23…:547

in Chrome's browser console.

That line is:
return fn.apply( context || this, args.concat( slice.call( arguments ) ) );

(In reply to Neil Kandalgaonkar from comment #13)

Potential fix in Gerrit as described in comment 11 above.
https://gerrit.wikimedia.org/r/#/c/158249/

That was merged four days ago so looks like it didn't help.

(In reply to Neil Kandalgaonkar from comment #11)

We can add a confirmCloseWindow() to the AJAX call. This is a feature in
UploadWizard (well, actually MediaWiki now, it got generalized) that will
ask the user to confirm if they want to navigate away. How does that sound?

Getting a modal dialog on a leave attempt will probably still kill any test. IMO the easy way to handle this is to expose the AJAX promise and have the test wait for it.

Tisza, we would have no problem negotiating a modal dialog in the test.

My concern is that even though UW tells the user that it will update their preferences, that may or may not actually happen, depending on local conditions.

neilk wrote:

Change 158240 https://gerrit.wikimedia.org/r/#/c/158240/ seems to have solved the issue with testing.

For just this one thing - the "side effect" of setting preferences with no UI acknowledgement at all - it will make the browser wait until AJAX calls are all completed. (In other cases, where we do expect some kind of UI response, we shouldn't use this trick; we should test if the UI updates appropriately.)

The problem seems to have finally gone away as of build #171.

https://integration.wikimedia.org/ci/job/browsertests-UploadWizard-commons.wikimedia.beta.wmflabs.org-linux-firefox-sauce/

(In reply to Chris McMahon from comment #16)

Tisza, we would have no problem negotiating a modal dialog in the test.

My concern is that even though UW tells the user that it will update their
preferences, that may or may not actually happen, depending on local
conditions.

neilk wrote:

I don't think this is related to this bug. That's something to do with the logging or other events Mark's been adding to UW.

(In reply to Andre Klapper from comment #14)

Using
http://commons.wikimedia.beta.wmflabs.org/wiki/Special:
UploadWizard?debug=true and clicking the checkbox, I get

Validation error against schema UploadWizardTutorialActions: Unknown schema:
UploadWizardTutorialActions
load.
php?debug=true&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&ve
rsion=20140907T23…:547

in Chrome's browser console.

That line is:
return fn.apply( context || this, args.concat( slice.call( arguments ) ) );

(In reply to Neil Kandalgaonkar from comment #13)

Potential fix in Gerrit as described in comment 11 above.
https://gerrit.wikimedia.org/r/#/c/158249/

That was merged four days ago so looks like it didn't help.

neilk wrote:

I don't seem to have the power to mark this closed, but I think it's done.

neilk wrote:

BY THE POWER OF GRAYSKULL, I RESOLVE THIS BUG.