--- trunk/webwork2/lib/WeBWorK/ContentGenerator/Problem.pm	2005/07/04 13:18:47	3337
+++ trunk/webwork2/lib/WeBWorK/ContentGenerator/Problem.pm	2006/03/02 17:03:54	4031
@@ -1,7 +1,7 @@
 ################################################################################
 # WeBWorK Online Homework Delivery System
-# Copyright © 2000-2003 The WeBWorK Project, http://openwebwork.sf.net/
-# $CVSHeader: webwork-modperl/lib/WeBWorK/ContentGenerator/Problem.pm,v 1.171 2005/06/28 00:16:08 sh002i Exp $
+# Copyright © 2000-2006 The WeBWorK Project, http://openwebwork.sf.net/
+# $CVSHeader: webwork-modperl/lib/WeBWorK/ContentGenerator/Problem.pm,v 1.196 2006/03/02 16:50:39 apizer Exp $
 # 
 # This program is free software; you can redistribute it and/or modify it under
 # the terms of either: (a) the GNU General Public License as published by the
@@ -27,13 +27,13 @@
 use warnings;
 use CGI qw();
 use File::Path qw(rmtree);
+use WeBWorK::Debug;
 use WeBWorK::Form;
 use WeBWorK::PG;
 use WeBWorK::PG::ImageGenerator;
 use WeBWorK::PG::IO;
-use WeBWorK::Utils qw(writeLog writeCourseLog encodeAnswers decodeAnswers ref2string makeTempDirectory);
+use WeBWorK::Utils qw(readFile writeLog writeCourseLog encodeAnswers decodeAnswers ref2string makeTempDirectory path_is_subdir sortByName);
 use WeBWorK::DB::Utils qw(global2user user2global findDefaults);
-use WeBWorK::Timing;
 use URI::Escape;
 
 use WeBWorK::Utils::Tasks qw(fake_set fake_problem);
@@ -85,6 +85,10 @@
 # 
 #     ($self, $User, $EffectiveUser, $Set, $Problem)
 
+# Note that significant parts of the "can" methods are lifted into the 
+# GatewayQuiz module.  It isn't direct, however, because of the necessity
+# of dealing with versioning there.
+
 sub can_showOldAnswers {
 	#my ($self, $User, $EffectiveUser, $Set, $Problem) = @_;
 	
@@ -170,10 +174,26 @@
 sub after   { return time >= $_[0] }
 sub between { my $t = time; return $t > $_[0] && $t < $_[1] }
 
+# Reset the default in some cases
+sub set_showOldAnswers_default {
+	my ($self, $ce, $userName, $authz, $set) = @_;
+	# these people always use the system/course default, so don't
+	# override the value of ...->{showOldAnswers}
+	return if $authz->hasPermissions($userName, "can_always_use_show_old_answers_default");
+	# this person should always default to 0
+	$ce->{pg}->{options}->{showOldAnswers} = 0
+		unless ($authz->hasPermissions($userName, "can_show_old_answers_by_default"));
+	# we are after the due date, so default to not showing it
+	$ce->{pg}->{options}->{showOldAnswers} = 0 if $set->{due_date} && after($set->{due_date});
+}
+
 ################################################################################
 # output utilities
 ################################################################################
 
+# Note: the substance of attemptResults is lifted into GatewayQuiz.pm,
+# with some changes to the output format
+
 sub attemptResults {
 	my $self = shift;
 	my $pg = shift;
@@ -217,17 +237,20 @@
 	my $fully = '';
 	my @tableRows = ( $header );
 	my $numCorrect = 0;
+	my $numBlanks  =0;
+	my $tthPreambleCache;
 	foreach my $name (@answerNames) {
 		my $answerResult  = $pg->{answers}->{$name};
 		my $studentAnswer = $answerResult->{student_ans}; # original_student_ans
 		my $preview       = ($showAttemptPreview
-		                    	? $self->previewAnswer($answerResult, $imgGen)
+		                    	? $self->previewAnswer($answerResult, $imgGen, \$tthPreambleCache)
 		                    	: "");
 		my $correctAnswer = $answerResult->{correct_ans};
 		my $answerScore   = $answerResult->{score};
 		my $answerMessage = $showMessages ? $answerResult->{ans_message} : "";
 		$answerMessage =~ s/\n/<BR>/g;
 		$numCorrect += $answerScore >= 1;
+		$numBlanks++ unless $studentAnswer =~/\S/ || $answerScore >= 1;   # unless student answer contains entry
 		my $resultString = $answerScore >= 1 ? "correct" :
 		                   $answerScore > 0  ? int($answerScore*100)."% correct" :
                                                        "incorrect";
@@ -256,28 +279,38 @@
 #	my $summary = "On this attempt, you answered $numCorrect out of "
 #		. scalar @answerNames . " $numIncorrectNoun correct, for a score of $scorePercent.";
 	my $summary = ""; 
-	if (scalar @answerNames == 1) {
-			if ($numCorrect == scalar @answerNames) {
-				$summary .= CGI::div({class=>"ResultsWithoutError"},"The above answer is correct.");
-			 } else {
-			 	 $summary .= CGI::div({class=>"ResultsWithError"},"The above answer is NOT ${fully}correct.");
-			 }
+	unless (defined($problemResult->{summary}) and $problemResult->{summary} =~ /\S/) {
+		if (scalar @answerNames == 1) {  #default messages
+				if ($numCorrect == scalar @answerNames) {
+					$summary .= CGI::div({class=>"ResultsWithoutError"},"The above answer is correct.");
+				 } else {
+					 $summary .= CGI::div({class=>"ResultsWithError"},"The above answer is NOT ${fully}correct.");
+				 }
+		} else {
+				if ($numCorrect == scalar @answerNames) {
+					$summary .= CGI::div({class=>"ResultsWithoutError"},"All of the above answers are correct.");
+				 } 
+				 unless ($numCorrect + $numBlanks == scalar( @answerNames)) {
+					$summary .= CGI::div({class=>"ResultsWithError"},"At least one of the above answers is NOT ${fully}correct.");
+				 }
+				 if ($numBlanks) {
+					my $s = ($numBlanks>1)?'':'s';
+					$summary .= CGI::div({class=>"ResultsAlert"},"$numBlanks of the questions remain$s unanswered.");
+				 }
+		}
 	} else {
-			if ($numCorrect == scalar @answerNames) {
-				$summary .= CGI::div({class=>"ResultsWithoutError"},"All of the above answers are correct.");
-			 } else {
-			 	 $summary .= CGI::div({class=>"ResultsWithError"},"At least one of the above answers is NOT ${fully}correct.");
-			 }
+		$summary = $problemResult->{summary};   # summary has been defined by grader
 	}
-	
 	return
 		CGI::table({-class=>"attemptResults"}, CGI::Tr(\@tableRows))
-		. ($showSummary ? CGI::p({class=>'emphasis'},$summary) : "");
+		. ($showSummary ? CGI::p({class=>'attemptResultsSummary'},$summary) : "");
 }
 
 
+# Note: previewAnswer is lifted into GatewayQuiz.pm
+
 sub previewAnswer {
-	my ($self, $answerResult, $imgGen) = @_;
+	my ($self, $answerResult, $imgGen, $tthPreambleCache) = @_;
 	my $ce            = $self->r->ce;
 	my $effectiveUser = $self->{effectiveUser};
 	my $set           = $self->{set};
@@ -296,9 +329,30 @@
 	if ($displayMode eq "plainText") {
 		return $tex;
 	} elsif ($displayMode eq "formattedText") {
+		
+		# read the TTH preamble, or use the cached copy passed in from the caller
+		my $tthPreamble='';
+		if (defined $$tthPreambleCache) {
+			$tthPreamble = $$tthPreambleCache;
+		} else {
+			my $tthPreambleFile = $ce->{courseDirs}->{templates} . "/tthPreamble.tex";
+			if (-r $tthPreambleFile) {
+				$tthPreamble = readFile($tthPreambleFile);
+				# thanks to Jim Martino. each line in the definition file should end with
+				#a % to prevent adding supurious paragraphs to output:
+				$tthPreamble =~ s/(.)\n/$1%\n/g;
+				# solves the problem if the file doesn't end with a return:
+				$tthPreamble .="%\n";
+				# store preamble in cache:
+				$$tthPreambleCache = $tthPreamble;
+			} else {
+			}
+		}
+		
+		# construct TTH command line
 		my $tthCommand = $ce->{externalPrograms}->{tth}
-			. " -L -f5 -r 2> /dev/null <<END_OF_INPUT; echo > /dev/null\n"
-			. "\\(".$tex."\\)\n"
+			. " -L -f5 -u -r  2> /dev/null <<END_OF_INPUT; echo > /dev/null\n"
+			. $tthPreamble . "\\[" . $tex . "\\]\n"
 			. "END_OF_INPUT\n";
 		
 		# call tth
@@ -306,8 +360,12 @@
 		if ($?) {
 			return "<b>[tth failed: $? $@]</b>";
 		} else {
+			#  avoid border problems in tables and remove unneeded initial <br>
+			$result =~ s/(<table [^>]*)>/$1 CLASS="ArrayLayout">/gi;
+			$result =~ s!\s*<br clear="all" />!!;
 			return $result;
 		}
+		
 	} elsif ($displayMode eq "images") {
 		$imgGen->add($tex);
 	} elsif ($displayMode eq "jsMath") {
@@ -332,6 +390,7 @@
 	my $userName = $r->param('user');
 	my $effectiveUserName = $r->param('effectiveUser');
 	my $key = $r->param('key');
+	my $editMode = $r->param("editMode");
 	
 	my $user = $db->getUser($userName); # checked
 	die "record for user $userName (real user) does not exist."
@@ -344,6 +403,17 @@
 	# obtain the merged set for $effectiveUser
 	my $set = $db->getMergedSet($effectiveUserName, $setName); # checked
 
+	$self->set_showOldAnswers_default($ce, $userName, $authz, $set);
+
+# gateway check here: we want to be sure that someone isn't trying to take 
+# a GatewayQuiz through the regular problem/homework mechanism, thereby 
+# circumventing the versioning, time limits, etc.
+	if (defined $set and defined $set->assignment_type and $set->assignment_type() =~ /gateway/) {
+		unless ($editMode eq "temporaryFile" and $authz->hasPermissions($userName, "modify_student_data")) {
+			die('Invalid access attempt: the Problem ContentGenerator was called for a GatewayQuiz assignment.'	);
+		}
+	}
+	
 	# Database fix (in case of undefined published values)
 	# this is only necessary because some people keep holding to ww1.9 which did not have a published field
 	# make sure published is set to 0 or 1
@@ -360,8 +430,6 @@
 	# obtain the merged problem for $effectiveUser
 	my $problem = $db->getMergedProblem($effectiveUserName, $setName, $problemNumber); # checked
 
-	my $editMode = $r->param("editMode");
-	
 	if ($authz->hasPermissions($userName, "modify_problem_sets")) {
 		# professors are allowed to fabricate sets and problems not
 		# assigned to them (or anyone). this allows them to use the
@@ -391,6 +459,7 @@
 			# if the global problem doesn't exist either, bail!
 			if(not defined $globalProblem) {
 				my $sourceFilePath = $r->param("sourceFilePath");
+				die "sourceFilePath is unsafe!" unless path_is_subdir($sourceFilePath, $ce->{courseDirs}->{templates});
 				# These are problems from setmaker.  If declared invalid, they won't come up
 				$self->{invalidProblem} = $self->{invalidSet} = 1 unless defined $sourceFilePath;
 #				die "Problem $problemNumber in set $setName does not exist" unless defined $sourceFilePath;
@@ -418,8 +487,8 @@
 		# if the caller is asking to override the source file, and
 		# editMode calls for a temporary file, do so
 		my $sourceFilePath = $r->param("sourceFilePath");
-		if (defined $sourceFilePath and 
-		    (not defined $editMode or $editMode eq "temporaryFile")) {
+		if (defined $editMode and $editMode eq "temporaryFile" and defined $sourceFilePath) {
+			die "sourceFilePath is unsafe!" unless path_is_subdir($sourceFilePath, $ce->{courseDirs}->{templates});
 			$problem->source_file($sourceFilePath);
 		}
 		
@@ -436,8 +505,9 @@
 		my $publishedClass = ($set->published) ? "Published" : "Unpublished";
 		my $publishedText = ($set->published) ? "visible to students." : "hidden from students.";
 		$self->addmessage(CGI::p("This set is " . CGI::font({class=>$publishedClass}, $publishedText)));
-	} else {
-	
+
+  # test for additional set validity if it's not already invalid
+        } else {
 		# A set is valid if it exists and if it is either published or the user is privileged.
 		$self->{invalidSet} = !(defined $set and ($set->published || $authz->hasPermissions($userName, "view_unpublished_sets")));
 		$self->{invalidProblem} = !(defined $problem and ($set->published || $authz->hasPermissions($userName, "view_unpublished_sets")));
@@ -489,8 +559,10 @@
 	# there is no way to override this.  Probably this is ok for the last three options, but it was definitely not ok for showing
 	# saved answers which is normally on, but you want to be able to turn it off!  This section should be moved to ContentGenerator
 	# so that you can set these options anywhere.  We also need mechanisms for making them sticky.
+	# Note: ProblemSet and ProblemSets might set showOldAnswers to '', which
+	#       needs to be treated as if it is not set.
 	my %want = (
-		showOldAnswers     => defined($r->param("showOldAnswers")) ? $r->param("showOldAnswers")  : $ce->{pg}->{options}->{showOldAnswers},
+		showOldAnswers     => (defined($r->param("showOldAnswers")) and $r->param("showOldAnswers") ne '') ? $r->param("showOldAnswers")  : $ce->{pg}->{options}->{showOldAnswers},
 		showCorrectAnswers => $r->param("showCorrectAnswers") || $ce->{pg}->{options}->{showCorrectAnswers},
 		showHints          => $r->param("showHints")          || $ce->{pg}->{options}->{showHints},
 		showSolutions      => $r->param("showSolutions")      || $ce->{pg}->{options}->{showSolutions},
@@ -538,7 +610,7 @@
 	
 	##### translation #####
 
-	$WeBWorK::timer->continue("begin pg processing") if defined($WeBWorK::timer);
+	debug("begin pg processing");
 	my $pg = WeBWorK::PG->new(
 		$ce,
 		$effectiveUser,
@@ -556,7 +628,7 @@
 		},
 	);
 	
-	$WeBWorK::timer->continue("end pg processing") if defined($WeBWorK::timer);
+	debug("end pg processing");
 	
 	##### fix hint/solution options #####
 	
@@ -590,27 +662,27 @@
 	return $self->{pg}->{head_text} if $self->{pg}->{head_text};
 }
 
-# sub options {
-# 	my ($self) = @_;
-# 	warn "doing options in Problem";
-# 	return "" if $self->{invalidProblem};
-# 	my $sourceFilePathfield = '';
-#         if($self->r->param("sourceFilePath")) {
-# 		$sourceFilePathfield = CGI::hidden(-name => "sourceFilePath", 
-#                                                    -value => $self->r->param("sourceFilePath"));
-# 	}
-# 	
-# 	return join("",
-# 		CGI::start_form("POST", $self->{r}->uri),
-# 		$self->hidden_authen_fields,
-# 		$sourceFilePathfield,
-# 		CGI::hr(), 
-# 		CGI::start_div({class=>"viewOptions"}),
-# 		$self->viewOptions(),
-# 		CGI::end_div(),
-# 		CGI::end_form()
-# 	);
-# }
+sub options {
+	my ($self) = @_;
+	#warn "doing options in Problem";
+	
+	# don't show options if we don't have anything to show
+	return "" if $self->{invalidSet} or $self->{invalidProblem};
+	return "" unless $self->{isOpen};
+	
+	my $displayMode = $self->{displayMode};
+	my %can = %{ $self->{can} };
+	
+	my @options_to_show = "displayMode";
+	push @options_to_show, "showOldAnswers" if $can{showOldAnswers};
+	push @options_to_show, "showHints" if $can{showHints};
+	push @options_to_show, "showSolutions" if $can{showSolutions};
+	
+	return $self->optionsMacro(
+		options_to_show => \@options_to_show,
+		extra_params => ["editMode", "sourceFilePath"],
+	);
+}
 
 sub siblings {
 	my ($self) = @_;
@@ -626,9 +698,11 @@
 	my $eUserID = $r->param("effectiveUser");
 	my @problemIDs = sort { $a <=> $b } $db->listUserProblems($eUserID, $setID);
 	
-	print CGI::start_ul({class=>"LinksMenu"});
-	print CGI::start_li();
-	print CGI::span({style=>"font-size:larger"}, "Problems");
+	print CGI::start_div({class=>"info-box", id=>"fisheye"});
+	print CGI::h2("Problems");
+	#print CGI::start_ul({class=>"LinksMenu"});
+	#print CGI::start_li();
+	#print CGI::span({style=>"font-size:larger"}, "Problems");
 	print CGI::start_ul();
 
 	foreach my $problemID (@problemIDs) {
@@ -642,8 +716,9 @@
 	}
 	
 	print CGI::end_ul();
-	print CGI::end_li();
-	print CGI::end_ul();
+	#print CGI::end_li();
+	#print CGI::end_ul();
+	print CGI::end_div();
 	
 	return "";
 }
@@ -691,7 +766,11 @@
 		push @links, "Next Problem", "", "navNext";
 	}
 	
-	my $tail = "&displayMode=".$self->{displayMode}."&showOldAnswers=".$self->{will}->{showOldAnswers};
+	my $tail = "";
+	
+	$tail .= "&displayMode=".$self->{displayMode} if defined $self->{displayMode};
+	$tail .= "&showOldAnswers=".$self->{will}->{showOldAnswers}
+		if defined $self->{will}->{showOldAnswers};
 	return $self->navMacro($args, $tail, @links);
 }
 
@@ -699,7 +778,7 @@
 	my ($self) = @_;
 
 	# using the url arguments won't break if the set/problem are invalid
-	my $setID = $self->r->urlpath->arg("setID");
+	my $setID = WeBWorK::ContentGenerator::underscore2nbsp($self->r->urlpath->arg("setID"));
 	my $problemID = $self->r->urlpath->arg("problemID");
 	
 	return "$setID: Problem $problemID";
@@ -717,7 +796,7 @@
 
 	if ($self->{invalidSet}) {
 		return CGI::div({class=>"ResultsWithError"},
-			CGI::p("The selected problem set (" . $urlpath->arg("setID") . ") is not a valid set for " . $r->param("effectiveUser") . "."));
+			CGI::p("The selected homework set (" . $urlpath->arg("setID") . ") is not a valid set for " . $r->param("effectiveUser") . "."));
 	}	
 	
 	if ($self->{invalidProblem}) {
@@ -727,7 +806,7 @@
 
 	unless ($self->{isOpen}) {
 		return CGI::div({class=>"ResultsWithError"},
-			CGI::p("This problem is not available because the problem set that contains it is not yet open."));
+			CGI::p("This problem is not available because the homework set that contains it is not yet open."));
 	}
 	# unpack some useful variables
 	my $set             = $self->{set};
@@ -756,19 +835,23 @@
 		my $editorPage = $urlpath->newFromModule("WeBWorK::ContentGenerator::Instructor::PGProblemEditor",
 			courseID => $courseName, setID => $set->set_id, problemID => $problem->problem_id);
 		my $editorURL = $self->systemLink($editorPage, params=>$forced_field);
-		$editorLink = CGI::a({href=>$editorURL}, "Edit this problem");
+		$editorLink = CGI::p(CGI::a({href=>$editorURL,target =>'WW_Editor'}, "Edit this problem"));
 	}
 	
 	##### translation errors? #####
 
 	if ($pg->{flags}->{error_flag}) {
-		print $self->errorOutput($pg->{errors}, $pg->{body_text});
+		if ($authz->hasPermissions($user, "view_problem_debugging_info")) {
+			print $self->errorOutput($pg->{errors}, $pg->{body_text});
+		} else {
+			print $self->errorOutput($pg->{errors}, "You do not have permission to view the details of this error.");
+		}
 		print $editorLink;
 		return "";
 	}
 	
 	##### answer processing #####
-	$WeBWorK::timer->continue("begin answer processing") if defined($WeBWorK::timer);
+	debug("begin answer processing");
 	# if answers were submitted:
 	my $scoreRecordedMessage;
 	my $pureProblem;
@@ -831,7 +914,7 @@
 				);
 			} else {
 				if (before($set->open_date) or after($set->due_date)) {
-					$scoreRecordedMessage = "Your score was not recorded because this problem set is closed.";
+					$scoreRecordedMessage = "Your score was not recorded because this homework set is closed.";
 				} else {
 					$scoreRecordedMessage = "Your score was not recorded.";
 				}
@@ -851,7 +934,7 @@
 			# FIXME  this is the line 552 error.  make sure original student ans is defined.
 			# The fact that it is not defined is probably due to an error in some answer evaluator.
 			# But I think it is useful to suppress this error message in the log.
-			foreach (sort keys %answerHash) {
+			foreach (sortByName(undef, keys %answerHash)) {
 				my $orig_ans = $answerHash{$_}->{original_student_ans};
 				my $student_ans = defined $orig_ans ? $orig_ans : '';
 				$answerString  .= $student_ans."\t";
@@ -872,7 +955,7 @@
 		}
 	}
 	
-	$WeBWorK::timer->continue("end answer processing") if defined($WeBWorK::timer);
+	debug("end answer processing");
 	
 	##### output #####
 	# custom message for editor
@@ -923,6 +1006,7 @@
 	print CGI::start_div({class=>"problem"});
 	print CGI::p($pg->{body_text});
 	print CGI::p(CGI::b("Note: "), CGI::i($pg->{result}->{msg})) if $pg->{result}->{msg};
+	print $editorLink; # this is empty unless it is appropriate to have an editor link.
 	print CGI::end_div();
 	
 	print CGI::start_p();
@@ -993,30 +1077,34 @@
 	if (before($set->open_date) or after($set->due_date)) {
 		$setClosed = 1;
 		if (before($set->open_date)) {
-			$setClosedMessage = "This problem set is not yet open.";
+			$setClosedMessage = "This homework set is not yet open.";
 		} elsif (after($set->due_date)) {
-			$setClosedMessage = "This problem set is closed.";
+			$setClosedMessage = "This homework set is closed.";
 		}
 	}
 	#if (before($set->open_date) or after($set->due_date)) {
 	#	$setClosed = 1;
-	#	$setClosedMessage = "This problem set is closed.";
+	#	$setClosedMessage = "This homework set is closed.";
 	#	if ($authz->hasPermissions($user, "view_answers")) {
 	#		$setClosedMessage .= " However, since you are a privileged user, additional attempts will be recorded.";
 	#	} else {
 	#		$setClosedMessage .= " Additional attempts will not be recorded.";
 	#	}
 	#}
-	
-	my $notCountedMessage = ($problem->value) ? "" : "(This problem will not count towards your grade.)";
-	print CGI::p(
-		$submitAnswers ? $scoreRecordedMessage . CGI::br() : "",
-		"You have attempted this problem $attempts $attemptsNoun.", CGI::br(),
-		$problem->attempted
-			? "Your recorded score is $lastScore.  $notCountedMessage" . CGI::br()
-			: "",
-		$setClosed ? $setClosedMessage : "You have $attemptsLeft $attemptsLeftNoun remaining."
-	);
+	unless (defined( $pg->{state}->{state_summary_msg}) and $pg->{state}->{state_summary_msg}=~/\S/) {
+		my $notCountedMessage = ($problem->value) ? "" : "(This problem will not count towards your grade.)";
+		print CGI::p(
+			$submitAnswers ? $scoreRecordedMessage . CGI::br() : "",
+			"You have attempted this problem $attempts $attemptsNoun.", CGI::br(),
+			$submitAnswers ?"You received a score of ".sprintf("%.0f%%", $pg->{result}->{score} * 100)." for this attempt.".CGI::br():'',
+			$problem->attempted
+				? "Your overall recorded score is $lastScore.  $notCountedMessage" . CGI::br()
+				: "",
+			$setClosed ? $setClosedMessage : "You have $attemptsLeft $attemptsLeftNoun remaining."
+		);
+	}else {
+		print CGI::p($pg->{state}->{state_summary_msg});
+	}
 	print CGI::end_div();
 		
 	# save state for viewOptions
@@ -1076,30 +1164,38 @@
 			CGI::endform();
 	}
 	
-	# feedback form url
-	my $feedbackPage = $urlpath->newFromModule("WeBWorK::ContentGenerator::Feedback",
-		courseID => $courseName);
-	my $feedbackURL = $self->systemLink($feedbackPage, authen => 0); # no authen info for form action
-	
-	#print feedback form
-	print
-		CGI::start_form(-method=>"POST", -action=>$feedbackURL),"\n",
-		$self->hidden_authen_fields,"\n",
-		CGI::hidden("module",             __PACKAGE__),"\n",
-		CGI::hidden("set",                $set->set_id),"\n",
-		CGI::hidden("problem",            $problem->problem_id),"\n",
-		CGI::hidden("displayMode",        $self->{displayMode}),"\n",
-		CGI::hidden("showOldAnswers",     $will{showOldAnswers}),"\n",
-		CGI::hidden("showCorrectAnswers", $will{showCorrectAnswers}),"\n",
-		CGI::hidden("showHints",          $will{showHints}),"\n",
-		CGI::hidden("showSolutions",      $will{showSolutions}),"\n",
-		CGI::p({-align=>"left"},
-			CGI::submit(-name=>"feedbackForm", -label=>"Email instructor")
-		),
-		CGI::endform(),"\n";
-	
-	# FIXME print editor link
-	print $editorLink;   #empty unless it is appropriate to have an editor link.
+	## feedback form url
+	#my $feedbackPage = $urlpath->newFromModule("WeBWorK::ContentGenerator::Feedback",
+	#	courseID => $courseName);
+	#my $feedbackURL = $self->systemLink($feedbackPage, authen => 0); # no authen info for form action
+	#
+	##print feedback form
+	#print
+	#	CGI::start_form(-method=>"POST", -action=>$feedbackURL),"\n",
+	#	$self->hidden_authen_fields,"\n",
+	#	CGI::hidden("module",             __PACKAGE__),"\n",
+	#	CGI::hidden("set",                $set->set_id),"\n",
+	#	CGI::hidden("problem",            $problem->problem_id),"\n",
+	#	CGI::hidden("displayMode",        $self->{displayMode}),"\n",
+	#	CGI::hidden("showOldAnswers",     $will{showOldAnswers}),"\n",
+	#	CGI::hidden("showCorrectAnswers", $will{showCorrectAnswers}),"\n",
+	#	CGI::hidden("showHints",          $will{showHints}),"\n",
+	#	CGI::hidden("showSolutions",      $will{showSolutions}),"\n",
+	#	CGI::p({-align=>"left"},
+	#		CGI::submit(-name=>"feedbackForm", -label=>"Email instructor")
+	#	),
+	#	CGI::endform(),"\n";
+	
+	print $self->feedbackMacro(
+		module             => __PACKAGE__,
+		set                => $self->{set}->set_id,
+		problem            => $problem->problem_id,
+		displayMode        => $self->{displayMode},
+		showOldAnswers     => $will{showOldAnswers},
+		showCorrectAnswers => $will{showCorrectAnswers},
+		showHints          => $will{showHints},
+		showSolutions      => $will{showSolutions},
+	);
 	
 	print CGI::end_div();