@Mvolz The quiz does vanish when it is submitted while editing, but does submitting a quiz while editing intended behavior ?
index.php has GET parameter action=edit when editing is being done, while quiz extension uses POST method to submit the answers whereas when normally quiz is submitted index.php has GET parameter action=view.
This might be due to how index.php handles request while having different parameters and might be out of power for Quiz extension.

It should be an intended behavior: one wants to check if the quiz works. The extension should use AJAX for submitting quizzes, which provides better performance for every use case while resolving this bug.

index.php handles all requests equally in this sense, the problem is that the quiz submits a request which doesn't have the required POST parameters such as the content of the edit form or the edit token.

A quiz that is not working i.e having bad syntax will show syntax error in preview itself and won't need to be submitted.
IMHO a quiz works if it doesn't have any syntax error, if there is an exception then do provide how to reproduce that.

Currently the way the extension is setup this should not be considered as bug, but rather a feature request to add AJAX to improve performance.

OK, than consider it as a feature request. Should this task's description be modified accordingly, or is it better to open a new task for it?

I checked the code for the extension which suggests that this is a invalid bug.

Code snippet from ext.quiz.js

// Disable the submit button if the page is in preview mode
	if( input[j].type === 'submit' && document.editform ) {
		input[j].disabled = true;
	 }

The Javascript is responsible to disable submit button so that the quiz is not submitted while preview or edit.
If you try editing on any wikiversity site containing quiz, example - https://en.wikiversity.org/wiki/Help:Quiz#The_quiz_tag, the submit button is disabled.

This might have been caused due to disabled javascript in local system.

It's not invalid, the issue is larger than it seemed to be: the JavaScript tries to work browser differences around on its own without using jQuery or mw.hook. Therefore, using live preview, not any code runs. It shouldn't do workarounds, instead of lines 141 to 153, it should contain simply

mw.hook( 'wikipage.content' ).add( prepareQuiz );

(And AFAIK, JavaScript for MediaWiki should use tabs for indentation, not two spaces.)
I may rewrite this script using modern techniques (jQuery and ResourceLoader) if nobody else wants to do it.

I think updating the JS to more moderns standards and using mw.hook is definitely in remit (this was written literally 10 years ago, I think JQuery is only a year older :P).

I'm actually not quite sure I understand what the bug is though; is this specifically for the preview page then? If that's the case it would be a nice feature to be able to test out the Quiz in the preview area rather than having to save before testing. I'm not sure what the original reason for disabling it was though, maybe it wasn't working properly that way.

The submit button was disabled because submitting a quiz creates a POST request to check the result—without the edit data (edit box content, token etc.), which means that submitting the quiz loses all edit data.

Change 362793 had a related patch set uploaded (by Tacsipacsi; owner: Tacsipacsi):
[mediawiki/extensions/Quiz@master] Use CSS and jQuery, JavaScript hook instead of window.onload

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

In T146370#3335700, @Tacsipacsi wrote:

It should be an intended behavior: one wants to check if the quiz works. The extension should use AJAX for submitting quizzes, which provides better performance for every use case while resolving this bug.

index.php handles all requests equally in this sense, the problem is that the quiz submits a request which doesn't have the required POST parameters such as the content of the edit form or the edit token.

JS is disabled in mobile, so we'd need to have a fallback for mobile (i.e. use REST rather than AJAX) if we choose to do that. That'd likely be a separate task.

In T146370#3397331, @Mvolz wrote:

JS is disabled in mobile, so we'd need to have a fallback for mobile (i.e. use REST rather than AJAX) if we choose to do that.

Why would JavaScript be disabled on mobile? Mobile has to be manually enabled for all modules—this is what I did in this patch for this extension’s modules (among other things).

It is automatically disabled by design because javascript can make mobile use difficult:

https://www.mediawiki.org/wiki/ResourceLoader/Writing_a_MobileFrontend_friendly_ResourceLoader_module

"Why does mobile turn my ResourceLoader modules off by default?

Note good web practice states that you should have good fallbacks for non-JavaScript users. There are variety of reasons why you should care about non-JavaScript experiences and if the crucial functionality of your feature is not working on mobile, it's probably time to go back to basics and make sure it does.

There is a good reason why that we decided to turn ResourceLoader modules off by default. "We want you to think about mobile."

By breaking your toy, we force you to make a decision. Namely that decision is 1) Do I do the bare minimum to get it working 2) Do I rethink my feature for mobile 3) Do I support only desktop mode (and make that obvious to the user) or 4) Do I leave my feature broken.

Provided your answer is not 4, your answer is acceptable :-)

We broke `your toy` because:

By breaking your feature it forces you to think about mobile experiences. If you've noticed it is broken, you care and can think about it. Making breakage less obvious could have the adverse effect of your feature staying broken (given it is relatively trivial in most cases to get it work).
Some modules may not have been built with mobile in mind and could cause a mobile browser to crash due to heavy CPU usage. e.g. a maps based application.
An interface built ten years ago may have been built with desktop devices in mind, and may require a desktop experience e.g. it might optimise for viewing with a resolution of at least 600px
Sometimes a completely different experience is warranted. Using targets you can decide to give a completely different experience to your mobile site users. For instance a photo curation tool might work better on mobile devices if it has a swipe left/swipe right interface rather than a table experience on desktop.
Certain key interface elements may not be easily clickable on mobile - for instance a list of search results which is less than 40px (Haven't been designed for the fat finger)
Making a site mobile friendly is not necessarily a case of slapping some media queries on to it - another approach would be to make your site mobile first and rethink your feature altogether.
In mobile we care a lot about resources loaded. We are trying our darn hardest to shrink the size of assets down as far as possible to give our users in countries with slower connections the best possible experiences. As a result we only allow standard JavaScript libraries and standard styles for interface elements as decided by the ux standardisation project.
If your module uses jquery.ui - it's time to rewrite it. On mobile it would be irresponsible to load jquery.ui and oojs.ui when both libraries solve similar problems.
We don't load common scripts or MediaWiki:Common.js/css as these were designed without mobile in mind - many of the rules are not useful for mobile screens and greatly increase the payload. We were also not keen for existing users to have the mobile site explode in front of their faces due to assumptions made by these scripts e.g. I am on Vector and this DOM element is available."

I think that in this particular case, it's great to have the javascript, but it should remain turned off in mobile as it's really not necessary :).

I don’t understand you. Why should we disable JavaScript if we can enable it and it works perfectly well? Why should we drop the possibility of using AJAX? How would you check the answers without reloading the page if you have no JavaScript? I see the point in disabling modules on mobile by default as they may not work there, but in this case, I tested the modules (both CSS and JavaScript) and they work on mobile as intended. Also, it’s quite a small module: 1.5 kB compressed—the startup module is 28 kB.

@Mvolz, I forgot that I couldn’t make it load the modules on preview (i.e. the submit button wasn’t grayed out initially, only after loading the module manually using the browser console). I’m not familiar with PHP hooks, could you please take a look at it?

@Tascipasci thanks for the PR!

I'm not that familiar with php hooks either, not sure how much help I can be ^-^. @Harjotsingh @Reedy any ideas?

What's the question about hooks?

@Reedy: Using live preview, Quiz’s modules (one module in the master version, two in this patch) aren’t get loaded, thus the submit button remains enabled. When I load the module manually using the browser console, the patched module works, the 10-year-old original doesn’t. With the normal preview, both versions work fine without manually loading any modules.

That's not really a PHP hook question then, it's maybe a resource loader issue/question.

What's live preview?

Live preview is a core function (can be turned on at Preferences/Editing), which uses AJAX to get the preview/diff instead of traditional POST requests, and makes editing really faster. It has some annoying bugs, but I cannot remember any issues with loading extension modules.

In T146370#3420863, @Tacsipacsi wrote:

Live preview is a core function (can be turned on at Preferences/Editing), which uses AJAX to get the preview/diff instead of traditional POST requests, and makes editing really faster. It has some annoying bugs, but I cannot remember any issues with loading extension modules.

And does it work in normal preview?

If you remove the conditional wrapping whether the $wgOut->addModule calls are done... Does it work in live preview?

Normal preview works well. I tried to move $wgOut->addModule to Quiz.hooks.php to make sure it has no condition; normal preview still works, live still doesn’t. Maybe it’s because live preview uses API and $wgOut does nothing in this case?

@Reedy can you comment on enabling all of the JS in mobile? My feeling is that we shouldn't given that this doesn't add any critical functionality.

In T146370#3421208, @Tacsipacsi wrote:

Normal preview works well. I tried to move $wgOut->addModule to Quiz.hooks.php to make sure it has no condition; normal preview still works, live still doesn’t. Maybe it’s because live preview uses API and $wgOut does nothing in this case?

Possibly, I'm guessing that code path just doesn't get executed for one reason or another. Could be considered an MediaWiki-Action-API bug to some extent. Just some code path less executed...

In T146370#3455942, @Mvolz wrote:

@Reedy can you comment on enabling all of the JS in mobile? My feeling is that we shouldn't given that this doesn't add any critical functionality.

All of what JS? The Quiz ones? I think what you said before is right. Sure, it'd be nice, but it's not really a necessity

Mobile is very much a special snowflake

I resolved the live preview issue by using $this->mParser->getOutput() instead of $wgOut.

In T146370#3456378, @Reedy wrote:

Sure, it'd be nice, but it's not really a necessity

I don’t see why should this JavaScript be disabled on mobile. The mobile editor, sidebar etc. are neither necessary. Why are they enabled (furthermore, separately developed) on mobile if we want to give as little JavaScript on mobile as possible? Ideally everything would be possible without JavaScript, so we wouldn’t need any on mobile. Would be that better?

Ideally, everything should have a non JS fallback

Yes, I agree that everything should have a non-JS fallback. But that doesn’t mean that we shouldn’t use JavaScript anywhere.

I'm not sure what your point is.

Mobile purposely, and for various reasons limits js, and doesn't ship everything that it does on a desktop experience. It's by design. Bikeshedding over it seems well over and beyond the scope of this task

My problem is that I don’t understand why this particular module should be disabled while it works on mobile.

In T146370#3464082, @Tacsipacsi wrote:

My problem is that I don’t understand why this particular module should be disabled while it works on mobile.

Because MobileFrontend. It's nothing against yours specifically, it does it to most (if not all) js

You've already been given the reason why

In T146370#3397479, @Mvolz wrote:

It is automatically disabled by design because javascript can make mobile use difficult:

https://www.mediawiki.org/wiki/ResourceLoader/Writing_a_MobileFrontend_friendly_ResourceLoader_module

"Why does mobile turn my ResourceLoader modules off by default?

Note good web practice states that you should have good fallbacks for non-JavaScript users. There are variety of reasons why you should care about non-JavaScript experiences and if the crucial functionality of your feature is not working on mobile, it's probably time to go back to basics and make sure it does.

There is a good reason why that we decided to turn ResourceLoader modules off by default. "We want you to think about mobile."

By breaking your toy, we force you to make a decision. Namely that decision is 1) Do I do the bare minimum to get it working 2) Do I rethink my feature for mobile 3) Do I support only desktop mode (and make that obvious to the user) or 4) Do I leave my feature broken.

Provided your answer is not 4, your answer is acceptable :-)

We broke `your toy` because:

By breaking your feature it forces you to think about mobile experiences. If you've noticed it is broken, you care and can think about it. Making breakage less obvious could have the adverse effect of your feature staying broken (given it is relatively trivial in most cases to get it work).
Some modules may not have been built with mobile in mind and could cause a mobile browser to crash due to heavy CPU usage. e.g. a maps based application.
An interface built ten years ago may have been built with desktop devices in mind, and may require a desktop experience e.g. it might optimise for viewing with a resolution of at least 600px
Sometimes a completely different experience is warranted. Using targets you can decide to give a completely different experience to your mobile site users. For instance a photo curation tool might work better on mobile devices if it has a swipe left/swipe right interface rather than a table experience on desktop.
Certain key interface elements may not be easily clickable on mobile - for instance a list of search results which is less than 40px (Haven't been designed for the fat finger)
Making a site mobile friendly is not necessarily a case of slapping some media queries on to it - another approach would be to make your site mobile first and rethink your feature altogether.
In mobile we care a lot about resources loaded. We are trying our darn hardest to shrink the size of assets down as far as possible to give our users in countries with slower connections the best possible experiences. As a result we only allow standard JavaScript libraries and standard styles for interface elements as decided by the ux standardisation project.
If your module uses jquery.ui - it's time to rewrite it. On mobile it would be irresponsible to load jquery.ui and oojs.ui when both libraries solve similar problems.
We don't load common scripts or MediaWiki:Common.js/css as these were designed without mobile in mind - many of the rules are not useful for mobile screens and greatly increase the payload. We were also not keen for existing users to have the mobile site explode in front of their faces due to assumptions made by these scripts e.g. I am on Vector and this DOM element is available."

I think that in this particular case, it's great to have the javascript, but it should remain turned off in mobile as it's really not necessary :).

This text (which I’ve read at least three times before) explains why aren’t modules loaded by default on mobile. I thought you are against enabling this module by adding the mobile target. Am I wrong?

No.. If you want to enable it, do it (as long as it works! which you've already said it does). Doesn't make any difference to me :)

OK, then it was a misunderstanding. Actually, I created the patch before I thought you don’t want it on mobile, so it’s already enabled. :) Now it only has to be merged, but it can’t because of @Harjotsingh’s tests. Could you help removing obsolete tests? (I know about tests for the PHP code generating border colors—this is now done in CSS.)

Which obsolete tests?

Quiz::getColor is still well used...

It’s still used in the upstream code, but not in the patch. This is why the tests were created for it, and they prevent the patch from being merged.

15:42:44 Fatal error: Call to undefined method Quiz::getColor() in /home/jenkins/workspace/mwext-testextension-hhvm-jessie/src/extensions/Quiz/tests/phpunit/QuizTest.php on line 46

So, remove any usages of the function. In the case of tests, you remove the whole function that covers it. And the provider that gives it information. Should be as easy as deleting about 17 lines of php code to my reckoning

I removed this test, so now tests run without syntax error. However, they still fail because I don’t know what the original intent of the author was—there’s a $class variable, which was out of use until now, but had a value (numbers or words). I left them out from the expected values. @Harjotsingh, do you know anything about it?

They're CSS classes

I know that they are CSS classes, but neither of them is used by the extension’s CSS, and they didn’t appear in the result HTML until now.

Change 362793 merged by jenkins-bot:
[mediawiki/extensions/Quiz@master] Use CSS and jQuery, JavaScript hook instead of window.onload

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