163114 - Templatise all calls to DisplayError

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect, P2)

Description

[Gervase Markham [:gerv]]

DisplayError is deprecated; all calls to it need to be replaced with localisable
calls to ThrowUserError(). There are approximately 115 of them; this will
therefore be a tracking bug, as they get done in batches.

Gerv

Comment 1

[Gervase Markham [:gerv]]

Attachment: Patch A v.1

This patch does the first 19 calls to DisplayError, all in attachment.cgi.

Gerv

Comment 2

[Gervase Markham [:gerv]]

Attachment: Patch A v.2

Updated patch.

Gerv

Comment 3

[Gervase Markham [:gerv]]

Logs:

Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.22; previous revision: 1.21
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.10; previous revision: 1.9
done

Patch B coming up.

Gerv

Comment 4

[Gervase Markham [:gerv]]

Attachment: Patch B v.1

This patch does CGI.pl and buglist.cgi.

Gerv

Comment 5

[Tobias Burnus]

+  [% ELSIF error == "invalid_column_name_cookie" %]
+     column name <em>[% qfragment FILTER html %]</em>. 

+  ThrowCodeError("invalid_column_name_cookie");
How about
 ThrowCodeError("invalid_column_name_cookie", {'qfragment' => $fragment});

Otherwise it looks ok, i.e. r=burnus

Comment 6

[Gervase Markham [:gerv]]

Attachment: Patch B v.2

Version with review comments, to be checked in.

Gerv

Comment 7

[Gervase Markham [:gerv]]

Patch B checked in.

Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.13; previous revision: 1.12
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.11; previous revision: 1.10
done
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.197; previous revision: 1.196
done
Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.179; previous revision: 1.178
done

Gerv

Comment 8

[Tobias Burnus]

Comment on attachment 100375 [details] [diff] [review]
Patch B v.1

(close request to obsolete attachment 100375 [details] [diff] [review], the review fixed version, which
has been checked in, is attachment 100447 [details] [diff] [review])

Comment 9

[Gervase Markham [:gerv]]

Better late than never...

Gerv

Comment 10

[Gervase Markham [:gerv]]

Attachment: Patch C v.1

Patch C. I estimate that this and one more of a similar size should have the
job finished.

Gerv

Comment 11

[Tobias Burnus]

> +  [% ELSIF error == "invalid_component" %]
> +    [% title = "Invalid Component" %]
> +    The [% component FILTER html %] component doesn't exist in the 
> +    [% product FILTER html %] product.

> + $comp_id || ThrowCodeError("invalid_component");
You need to add the product and the component as argument.

>      # Add the quip 
>      my $comment = $::FORM{"quip"};
> -    if (!$comment) {
> -        DisplayError("Please enter a quip in the text field.");
> -        exit();
> -    }
You don't check for this anymore. Please add something like
$comment || ThrowUserError(...)

r=burnus if you fix those.

Comment 12

[Gervase Markham [:gerv]]

CHANGE PROCESS_BUG.CGI:636.

> You don't check for this anymore.

The copy of the patch I'm looking at has
+    $comment || ThrowUserError("need_quip");
on line 322. :-) Good call on the other one, though - I completely missed that.

Fixed.

Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.13; previous revision: 1.12
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.15; previous revision: 1.14
done
Checking in createaccount.cgi;
/cvsroot/mozilla/webtools/bugzilla/createaccount.cgi,v  <--  createaccount.cgi
new revision: 1.26; previous revision: 1.25
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.75; previous revision: 1.74
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.68; previous revision: 1.67
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.152; previous revision: 1.151
done
Checking in quips.cgi;
/cvsroot/mozilla/webtools/bugzilla/quips.cgi,v  <--  quips.cgi
new revision: 1.14; previous revision: 1.13
done

Gerv

Comment 13

[Gervase Markham [:gerv]]

Attachment: Patch D v.1

This is it - the last patch, the final removal of DisplayError.

Gerv

Comment 14

[Tobias Burnus]

> -    DisplayError("Unknown action: " . html_quote($action));
> +    ThrowCodeError("unknown_action");
You may want to change this to
  ThrowCodeError("unknown_action", { 'action' => $action});
to include more information.

> -            if ($prodcount{$prod} > $::prodmaxvotes{$prod}) {
> -                DisplayError("You may only use at most $::prodmaxvotes{$prod}
> +            ($prodcount{$prod} > $::prodmaxvotes{$prod})
> +              || ThrowUserError("too_many_votes_for_bug",
Are you sure that you don't revise the logic? Either use '&&' or '<='.
Additionally this should be "too_many_votes_for_product" not
"too_many_votes_for_bug".

> +     <tt>[% prod %]</tt> product, but you are trying to use [% votes %].
> +    Can't use [% field %] as a field name.
You should use a FILTER html here. If you fix those: r=burnus

Comment 15

[Gervase Markham [:gerv]]

All fixed! DisplayError() is no more.

Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v
 <--  user-error.html.tmpl
new revision: 1.14; previous revision: 1.13
done
Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v
 <--  code-error.html.tmpl
new revision: 1.16; previous revision: 1.15
done
Checking in votes.cgi;
/cvsroot/mozilla/webtools/bugzilla/votes.cgi,v  <--  votes.cgi
new revision: 1.8; previous revision: 1.7
done
Checking in userprefs.cgi;
/cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v  <--  userprefs.cgi
new revision: 1.45; previous revision: 1.44
done
Checking in reports.cgi;
/cvsroot/mozilla/webtools/bugzilla/reports.cgi,v  <--  reports.cgi
new revision: 1.60; previous revision: 1.59
done
Checking in sanitycheck.cgi;
/cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v  <--  sanitycheck.cgi
new revision: 1.61; previous revision: 1.60
done
Checking in showdependencygraph.cgi;
/cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v  <-- 
showdependencygraph.cgi
new revision: 1.23; previous revision: 1.22
done
Checking in sidebar.cgi;
/cvsroot/mozilla/webtools/bugzilla/sidebar.cgi,v  <--  sidebar.cgi
new revision: 1.10; previous revision: 1.9
done
Checking in token.cgi;
/cvsroot/mozilla/webtools/bugzilla/token.cgi,v  <--  token.cgi
new revision: 1.13; previous revision: 1.12
done
Checking in describecomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/describecomponents.cgi,v  <-- 
describecomponents.cgi
new revision: 1.18; previous revision: 1.17
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.209; previous revision: 1.208
done
Checking in CGI.pl;
/cvsroot/mozilla/webtools/bugzilla/CGI.pl,v  <--  CGI.pl
new revision: 1.183; previous revision: 1.182
done
Checking in Bugzilla/Search.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Search.pm,v  <--  Search.pm
new revision: 1.20; previous revision: 1.19
done

Gerv

Comment 16

[Tobias Burnus]

Comment on attachment 101380 [details] [diff] [review]
Patch D v.1

(Close request. This has been checked in.)