Closed (fixed)
Project:
Quiz
Version:
6.x-2.x-dev
Component:
Documentation
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
1 Feb 2007 at 06:36 UTC
Updated:
22 Jun 2014 at 16:17 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
wmostrey commentedhttp://mostrey.be/files/mostrey/quiz-5.tar.gz
This is the updated version for Drupal 5.
Currently the only thing that's not 100% is the layout of the multichoice. The options and check/radioboxes are displayed below eachother, not in a table view. Still working on getting that to work.
I would suggest the current maintainer to branch the current version of HEAD as development version 4.7: I've been running it for some weeks in a production environment and it's working just fine. Feel free to add this version as the development version of Drupal 5. It will make bugtracker easier too.
Comment #2
wmostrey commentedOk I found a solution to this. The issue was that check_markup adds a
around the answer. Webchick pointed to look at the Poll module. They use check_plain. Since currently the Quiz module doesn't allow an input forum for the answers, this is the current fix, with which we currently have a fully functional Drupal 5 version of Quiz :)
Replace
$options[$key] = '
';
With
$options[$key] = ''. check_plain($answer['answer']) .'';
Comment #3
saml commentedNice that it's up running on 5! =)
Comment #4
webchickDoes that mean you tested it?
I need some folks to test this before I can commit it. :P
Comment #5
saml commentedI just installed it, and basic functionality seems to be working well.
I found one bug though, posted in: http://drupal.org/node/117595
I also noticed that HTML is not parsed for the answer labels when viewing a multichoice node, or taking a quiz, while it is on the result page. (I tried e.g. the
<sup></sup>tags), but that's maybe more of a feature request.Also the user message don't render HTML correctly. One example is:
I will report any new bugs that I find!
Keep up the good work!, thanks,
// Samuel
Comment #6
goose2000 commentedThis is great news - Thanks Quiz guys. I can now think about moving up to the D5 higher plain!
John A
Comment #7
riverfr0zen commentedNice work, wmostrey. Has this been committed to HEAD yet?
Just installed and it looks like most things are working well. Below are any issues I found:
Folllow up to post #2:
In addition to switching to check_plain, you will also need to change line 86 in quiz.module
drupal_set_html_head(theme('stylesheet_import', base_path() . drupal_get_path('module', 'quiz') .'/quiz.css'));
to
drupal_add_css(drupal_get_path('module', 'quiz') .'/quiz.css', 'module', 'all');
in order to get the module css included in 5.x
Comment #8
wmostrey commentedThese features and bug fixes have been incorporated in the new version:
http://mostrey.be/files/mostrey/quiz-5--1.tar.gz
Comment #9
jinside commentedPatch for admin deleting of submissions. Delete process was using 4.7 style of forms. Renamed and created new function for 5.0 form style. Please test!
Thanks!
Comment #10
riverfr0zen commentedI've tested jinside's patch (quiz-5.1-admin-delete_0.patch above) and it fixes the bug as tested on wmostrey's post (http://drupal.org/node/114885#comment-203899)
Comment #11
riverfr0zen commentedDamn, I had this whole long post written up, and then hit alt-arrow left :(
Anyway, I spoke to webchick a little over IRC, and she stated that she is reluctant (for good reasons) to create a 5.x branch at this time (see http://drupal.org/node/93855). I personally would prefer a 5.x *dev* branch, since it doesn't seem like the people actively working on this are really interested in developing for 4.x anymore, but oh well.
This is fine, but brings in discrepancies in the normal dev workflow for this 5.x version. Namely, as there is no version context for this 'pseudo-branch', it is not possible for developers who are no longer interested in working with 4.x (HEAD) to submit/resolve bugs/patches for this new version, as those bugs/patches would very likely not be relevant to what is in HEAD.
To mitigate this issue, perhaps we could consider this thread a temporary station for this 5.x version. In any case, this seems to be what we are doing anyway. I suggest developers continue to post patches/diffs to this thread, and also adopt/test any fixes others post. Testing is most important, as that is what will lead to a 5.x branch soonest, which is what I think everyone would like.
As it seems wmostrey has taken it upon him/herself to compile changes posted here to and provide builds (have you? heh), I suggest also adopting these builds posted. I suggest these builds are considered to be the 'cumulative' versions, the versions that determine the 5.x Quiz's RTBC-ness, if you will.
Also, there are several patches posted for HEAD that seem to have languished for some time and are still to be tested. I suggest we adopt those patches into 5.x (please indicate where they come from and what they relate to).
Anyway, these are just my thoughts - would appreciate any of your thoughts/input.
Comment #12
hba commentedA 4.7 *dev* branch would be really nice as well, just to make the cutoff before 5.x.
Comment #13
saml commentedTested jinside's patch at http://drupal.org/node/114885#comment-205087
Deleting of quizzes in the admin page now works fine.
The issue http://drupal.org/node/114885#comment-194150 about HTML tags in quiz answers on taking quizzes still doesn't work though. HTML-tags (like
<sup></sup>) work when reviewing the issue, but not on taking it. Screenshot attached.Comment #14
webchick@hba: I'll be making a 4.7 branch once this issue is RTBC.
So again, please test and fix errors.
Comment #15
wmostrey commentedOkay I finally fixed the issue and brought back check_markup instead of check_plain:
http://mostrey.be/files/mostrey/quiz-5--2.tar.gz
This includes the patch at http://drupal.org/node/114885#comment-205087 and all previous patches.
I fixed it by changing
$options[$key] = ''. check_plain($answer['answer']) .'';
to
$options[$key] = '
';
and adding the following to quiz.css:
div.multichoice_answer_text p{
display:inline;
}
Comment #16
saml commentedhttp://mostrey.be/files/mostrey/quiz-5--2.tar.gz tested and works fine.
Answers now use the same input format as the main body. Precisely what I was looking for. Thanks!
Comment #17
deekayen commentedI think
package = Quizshould be added to the .info files.Comment #18
deekayen commentedField 'score' doesn't have a default value query: INSERT INTO quiz_result (rid, quiz_nid, uid, time_start) VALUES (1, 1, 1, 1172693258). Should default be 0? If so, quiz.install should havescore TINYINT NOT NULL DEFAULT 0for {quiz_result} creation.Comment #19
jinside commentedOk.. I am little confused can someone set me straight?
Doesnt "$options[$key] = ''. check_markup($answer['answer'], $node->format, FALSE) .'';" enter the possibility of XSS attack for a site that does not want any html content on their site?
Shouldn't it be "$options[$key] = ''. check_markup($answer['answer']) .'';" and then enable a filter with HTML content for questions and quiz module content types to display html content?
I could be off base.
Comment #20
riverfr0zen commentedPatch for the following reports above by deekayen:
http://drupal.org/node/114885#comment-205544
http://drupal.org/node/114885#comment-205556
Besides the items mentioned in those posts, I also did some general renaming of some literals to aid clarity/descriptiveness. You can diff the .module files to see those changes. Let me know what you think, and please do test. Thanks.
Comment #21
riverfr0zen commentedAs the name implies, the above to be patched over wmostrey's 5.2 build here:
http://drupal.org/node/114885#comment-205346
Comment #22
riverfr0zen commentedI noticed a bunch of mistakes with the default quiz name that I had made to the last patch I posted
(http://drupal.org/node/114885#comment-205872)
Here is a follow up patch that should fix those - please apply the patch above first, then apply this patch. There are also some additional fixes - mainly in the area of using the correct placeholders in the various calls to t().
Comment #23
riverfr0zen commented5.x port of the 4.x quiz_fatalerrors.patch at http://drupal.org/node/100066
Additionally, I made the following changes.
*Refactored so calls to arg(1) use constant QUIZ_NID_ARG.
*Re-enabled quiz_view override of the view, using 5.x recommendations.
*Added 'Edit' link to theme_quiz_question_table()
Comment #24
wflohr commentedi merged the patches from riverfr0zen
http://drupal.org/node/114885#comment-205872
http://drupal.org/node/114885#comment-205929
http://drupal.org/node/114885#comment-206387
and my own patch (showing quiz results doesn't use table aliases) and attached it to this post
Comment #25
Brendanmcp commentedHi everyone...I'm a Drupal newbie and I just thought I would give some feedback on my use of the Quiz module for Drupal 5. It works perfectly in IE for me (the admin), but not in Firefox and not in either browser for anonymous or authenticated users. I have all permissions set but for anonymous users it doesn't jump to the next question and for authenticated users it goes to the next question but I get the following in red:
Here is the site. http://votingspot.com/are-you-american-idol-expert
Thank you for all your work on this and I'm looking forward to using the final version!
Comment #26
webchickresetting status
Comment #27
EduardoMercovich commentedHello everybody.
I would like to contribute to this 5.x module. Since I can't code (I don't know how), I can test it. So -to us dummies that don't know about patches, code, programming, etc.- what package/version/tar-gz/whatever should I download and install?
Thanks in advance, and please understand my (limited) knowledge and position. :-)
Best regards...
Comment #28
riverfr0zen commentedHey there Eduardo - I'll try to put up an archive of latest stuff with everything merged (and some additional changes) later tonight.
Comment #29
riverfr0zen commentedEduardo - sorry it's late - here's an archive with the latest patches and some other minor fixes. Let me know if you have any problems. Would really appreciate if you (and anyone else) could test it out.
http://irfanbaig.com/devbin/quiz-5.3-riverfr0zen.tgz
Comment #30
curtisc@drupal.org commentedIs this going to be added into the CVS for the module?
Comment #31
webchickhttp://drupal.org/node/114885#comment-205331
Comment #32
wmostrey commentedI've been testing #29 and all works perfectly. I do have one comment: I was trying to add labels to this version, but it appears that, while checkboxes have a unique id, radio buttons are not provided one. I will try to fix this myself if I find the time next week but I believe this should be fixed before rolling out.
Comment #33
wmostrey commentedAn update on this: it seems to me the only way to add unique id's would be to call http://api.drupal.org/api/5/function/theme_radio. Perhaps we could include labals, that work for checkboxes but if a user want labels enabled for radios, he should add a snippet to his template.php? I don't know if that's acceptable. Or is there a way to call that function from within the module?
Comment #34
wmostrey commentedIt's a html input format's
<p></p>wrapping around the answer that stops the label from working. So this works for any other format, or in the case the input format wouldn't wrap the text. I think the cleanest option would be to say it's "by design". If everyone agrees: RTBC.Comment #35
add1sun commentedI tested this out briefly and seems like everything is basically working. Good job on moving everything up to 5!
That said, there are a lot of code style issues and I found a few instances of theme('placeholder') being passed into t() functions (or actually called from within the t()?!?) that led to screwy output. I'm also not a fan of all the new constants, particularly QUIZ_NID_ARG. Maybe it is just me, but I actually really prefer arg(1) in the code.
I hope to get more time to review code and test in the next few days. Is the idea that RTBC means "good enough" to not break a site, just so we can get the branch? It definitely needs some cleanup for a normal RTBC. I won't have time to clean and patch until next week but am more than willing to do that.
Comment #36
riverfr0zen commentedStill a little new to Drupal way of things - sorry about the QUIZ_NID_ARG - I think you're right on that one, since those values are not constant across function calls. My bad - I'll remove them in my next code release, unless someone beats me to it. That use of non-descriptive numbers still makes me wince though. It makes code more unreadable.
But I'd disagree about most of the others constant. Constants for perms, for e.g. allow ease of changing those perms in the future. Not only in the module in question, but across dependencies too. Especially in a component this young, but even later.
Using constants to replace arbitrary or volatile literals is quite useful. It is a basic tenet of refactoring and provides not just more readable, but also more easily modified code.
Comment #37
add1sun commentedHm, well I still think that things like ('QUIZ_DEFAULT_NAME', 'Quiz') are unnecessary since the default name of 'quiz' is pretty straight-forward and the chances of that being changed over time is pretty much slim to none I would imagine. Also, since it is a stored default variable people can (and will) easily change in the UI, I just don't see any real advantage here.
But those are just my thoughts and webchick can sort out yea or nay on that.
Comment #38
webchickI'd say if the thing works without majorly imploding your site, go ahead and mark RTBC and I'll branch. We can sort out auxiliary issues in subsequent patches. In general though, I agree with add1sun that we should use the standard "Drupal way" of dealing with things like configurable variables and URL arguments.
Comment #39
riverfr0zen commentedI feel that's quite an assumption you're making that something won't be changed over time (for e.g. I changed those values within the last couple of mods - they were lower case).
Even if the frequency of change is low, why wouldn't you code in a way that will make it easier when a change *is* required? It isn't as though adding a constant will overwhelm resources, nor should it break the application, because of the module name prefix (QUIZ_). I really can't see any downside to it.
Also, using a constant for a default literal has nothing to do with user configuration. It serves the developer, in terms of understanding code and ease of modification.
Of course, I don't want to break any framework conventions, and as I admitted, the node id constant was a silly mistake. But the rest of the stuff doesn't really break any conventions. Just seems like no one else has done it before. Unless I am missing some discourse/documentation (let me know where, if I am).
Comment #40
webchickI haven't looked at the code, but I assume that you're using that define so that people can change all instances of the word "quiz" in the UI to "test" or "banana."
The t() function in conjunction with the locale module is how this is generally done in Drupal. You simply create a new "English" language and replace your custom strings in there and you're good to go.
If there's been a big enough call to change this in the UI (I don't remember anyone but like one person suggesting it, but I also haven't been following closely), then you could also do something like add in an option on the quiz settings page for this (much like we allow people to change the title of the Title and Body fields on nodes).
But having it as a define strikes me as non-optimal; in order to change it, I have to go in and edit the code. When I update to the next version of quiz module, I have to remember my change and port it. I'd like people to be able to customize quiz module without having to change the source code.
Comment #41
riverfr0zen commentedNo, you're misunderstanding. As I said, it has nothing to do with user configuration - it's for developer purposes. Obviously, you wouldn't use a constant to define something for users to change. Generally, I hope one wouldn't even expect a user to open the source.
e.g. calls to:
variable_get('quiz_name', 'quiz') // Second param being the default value
were some things I changed to
variable_get('quiz_name', QUIZ_DEFAULT_NAME)
So, for developers, if you wanted to change the default value, all you need to do is modify the constant value. So much easier than going through the entire source.
Comment #42
add1sun commentedYeah, all I meant was that the default in the code is so straight-forward and it is just a default that ends up easily changed by the user on the settings page so - I guess I mean I don't see that there is a lot of fretting to be done on changing a variable_get default that makes a constant necessary. I'm not very experienced with code, so in the end it doesn't really matter to me, I'm just not used to it from other Drupal code I have looked at. I have no idea about whether it makes sense to use on constants on perms but I've never seen that anywhere else in Drupal code either, so it just strikes me as weird.
We can discuss/decide more on this once we get our branch. ;) My test site is not a smoldering ruin so I'd say the code as is works well enough so I'm marking it RTBC.
Comment #43
webchickCommitted and branched. Awesome work, folks. :) Thanks for all of your hard work!!
I've marked this issue fixed. Please post any additional weirdness clean-up bugs as separate issues.
wmostrey or riverfr0zen: if either of you (or both) are interested in taking on maintainership of this module, please post back here and I'll make it happen.
Comment #44
riverfr0zen commentedYeah, I wouldn't mind maintaining it - although, since I'm quite new to the Drupal world, it would be nice to have someone else co-maintaining too. Regardless, I certainly would like to be on the dev team.
Comment #45
wmostrey commentedI'd be happy to co-maintain it with riverfr0zen :)
Comment #46
webchickI've added wmostrey to the list, but riverfr0zen, it says you don't have a CVS account yet... please go to http://drupal.org/cvs-account and request one, saying that you wish to be a co-maintainer for Quiz module, and reference this issue.
How this co-maintainership has worked in the past is, when you find something you want to change, make an issue, attach a patch. As long as at least one other person looks at it and gives the thumbs-up, feel free to commit it. :)
Comment #47
riverfr0zen commentedthanks, did done that - just waiting till the application is processed.
Comment #48
riverfr0zen commentedHey webchick - looks like my CVS application came through, so whenever you get a chance ... thanks
Comment #49
webchickdone!
Comment #50
killes@www.drop.org commentedComment #51
gingerjoos commentedThe releases page for Quiz module ( http://drupal.org/node/26481/release ) does not seem to have the old (unsupported) Drupal 5 version. Am I looking at the wrong place? Where can I get the old Drupal 5 version?
Comment #52
mbutcher commentedIt was removed from the official downloads for security reasons (there is a known vulnerability that none of us has the time to patch). You are welcome to check it out from CVS, patch it up, and use it.