Last modified: 2014-09-10 01:11:03 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T72303, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 70303 - Skip tutorial does not set user preference in Chrome and Firefox
Skip tutorial does not set user preference in Chrome and Firefox
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
UploadWizard (Other open bugs)
unspecified
All All
: High normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: browser-test-bug
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-09-02 21:00 UTC by Chris McMahon
Modified: 2014-09-10 01:11 UTC (History)
9 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Chris McMahon 2014-09-02 21:00:04 UTC
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
Comment 1 Chris McMahon 2014-09-02 21:09:37 UTC
Update: happens in Firefox also it seems but sometimes the setting will actually take permanentlty
Comment 2 Chris McMahon 2014-09-02 21:40:07 UTC
I have reproduced this manually a number of times also.
Comment 3 Neil Kandalgaonkar 2014-09-02 22:48:38 UTC
Can't reproduce this.
Comment 4 Chris McMahon 2014-09-02 22:53:13 UTC
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
Comment 5 Rummana Yasmeen 2014-09-02 23:54:38 UTC
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
Comment 6 Neil Kandalgaonkar 2014-09-03 00:27:31 UTC
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.
Comment 7 Chris McMahon 2014-09-03 00:33:29 UTC
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"
Comment 8 Neil Kandalgaonkar 2014-09-03 00:55:19 UTC
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.
Comment 9 Neil Kandalgaonkar 2014-09-03 01:29:01 UTC
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.
Comment 10 Chris McMahon 2014-09-03 14:58:16 UTC
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.
Comment 11 Neil Kandalgaonkar 2014-09-03 21:11:57 UTC
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?
Comment 12 Chris McMahon 2014-09-03 21:31:30 UTC
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.
Comment 13 Neil Kandalgaonkar 2014-09-03 21:41:46 UTC
Potential fix in Gerrit as described in comment 11 above.

https://gerrit.wikimedia.org/r/#/c/158249/
Comment 14 Andre Klapper 2014-09-09 17:21:56 UTC
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.
Comment 15 Tisza Gergő 2014-09-09 19:50:54 UTC
(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.
Comment 16 Chris McMahon 2014-09-09 20:34:34 UTC
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.
Comment 17 Neil Kandalgaonkar 2014-09-09 21:52:37 UTC
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.
Comment 18 Neil Kandalgaonkar 2014-09-09 21:54:11 UTC
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.
Comment 19 Neil Kandalgaonkar 2014-09-09 21:55:19 UTC
I don't seem to have the power to mark this closed, but I think it's done.
Comment 20 Andre Klapper 2014-09-10 01:08:25 UTC
Neil: Now you can. :)
Comment 21 Neil Kandalgaonkar 2014-09-10 01:11:03 UTC
BY THE POWER OF GRAYSKULL, I RESOLVE THIS BUG.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links