Closed Bug 65316 Opened 19 years ago Closed 16 years ago

Typos on edit*.cgi.

Categories

(Bugzilla :: Administration, task, P4, trivial)

2.13

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: CodeMachine, Assigned: goobix)

Details

Attachments

(1 file, 4 obsolete files)

There are some trivial typos on the page after you add a component, product or
user.

What they are:

Back to the query page, edit more products or add components to this new
product..
Back to the index, edit more users or add another user..
Back to the query page or edit more keywords or add another keyword.
Back to the query page or edit more components or Add another component.

What they should be:

Back to the query page, edit more products or add components to this new
product. [ "." not ".." ]
Back to the index, edit more users or add another user. [ "." not ".." ]
Back to the query page or edit more keywords or add another keyword. [ ", " not
" or " ]
Back to the query page or edit more components or Add another component. [ ", "
not " or " + "add" not "Add" ]
editgroups.cgi is also consistent.  It says:

Back to the Main Bugs Page, Add another group or Back to the group list.

It uses capitals everywhere, and the whole text is hyperlinkified unlike the
others.
Whiteboard: 2.14
Whiteboard: 2.14 → 2.16
moving to real milestones...
Target Milestone: --- → Bugzilla 2.16
Priority: -- → P4
Whiteboard: 2.16
Moving to new Bugzilla product ...
Assignee: tara → justdave
Component: Bugzilla → Administration
Product: Webtools → Bugzilla
Version: other → 2.13
Taking ...
Assignee: justdave → matty
QA Contact: matty → jake
Status: NEW → ASSIGNED
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Attached patch Patch ver 1 (obsolete) — Splinter Review
Matty describes 4 errors in the original post and 1 error in the additional
comment.

Those errors and their corrections are related to:

1st -> editproducts.cgi
2nd -> editusers.cgi
3rd -> editkeywords.cgi
4th -> editcomponents.cgi
5th -> editgroups.cgi

(yes, the corrections for the 3rd and 4th are identical with the current wrong
version, but the description regarding what to fix is correct)

Sorry for the large patch. If you want it split up for each individual file,
I'll be happy to do it.

PutTrailer already adds the "or", "," or "." in the suitable places. So the
patch is mainly fixing that and deals with capitalization issues.


I thought about doing a template with those but it would involve more or less
the full templatization of 5 admin scripts which would fall beyond the purpose
of this bug. We can pick those up when we make the templates, IMO.
Attached patch Patch ver 2 (obsolete) — Splinter Review
I forgot about a capitalization fix in editcomponents.cgi.
Attachment #131814 - Attachment is obsolete: true
Attachment #131815 - Flags: review?(kiko)
Comment on attachment 131815 [details] [diff] [review]
Patch ver 2

Could you break those long lines to something closer to 72 chars, while you're
at it?

(Err, why is PutTrailer() copied nearly verbatim in  7 edit*.cgi files? Vladd,
can this be solved with the templatization that is ongoing -- bug 207211, for
instance, or should we open another bug?)

The linking isn't very standardized, I'm afraid -- certain places are in the
format <a>foo</a> my bar, others <a>foo my bar</a>. Should this be consistent?
Attachment #131815 - Flags: review?(kiko) → review-
Attached patch Patch ver 3 (obsolete) — Splinter Review
I've broke the lines around 72 chars, and I've moved towards the "<a>foo</a> my
bar" style.

Regarding PutTrailer redundancy, I think it will be solved together with
templatization efforts.
Attachment #131815 - Attachment is obsolete: true
Attachment #131858 - Flags: review?(kiko)
There are tabs in your patch, and every time you add a tab to a patch a kitten
is murdered! Go configure your editor ;-)
http://www.mozilla.org/hacking/mozilla-style-guide.html#Visual
Comment on attachment 131858 [details] [diff] [review]
Patch ver 3

I know you're not going to like this :-) but the use of the word "index" is
inconsistent with "main bugs page". You can place my rs= on that change
assuming the tabs are gone, since I don't want to be too nasty over a silly bug
like this one (which is going to become irrelevant as soon as we've
templatized, anyway).
Attachment #131858 - Flags: review?(kiko) → review+
Just clarifying (and now that I've done some grepping), I'm suggesting 

  s/main bugs page/index/

on its only occurence.
Attached patch Patch ver 4 (obsolete) — Splinter Review
Sorry for the delay.

(In reply to comment #10)
> There are tabs in your patch, and every time you add a tab to a patch a
kitten
> is murdered! Go configure your editor ;-)

Oops! Fixed. I did configure my "vim" editor some time ago, but auto-expand
tabs doesn't work. I'll search for some additional documentation and try again.
:)

(In reply to comment #11)
> I know you're not going to like this :-)

Why would someone dislike improvement suggestions? :-)

> which is going to become irrelevant as soon as we've
templatized, anyway

Yes, true. But when we do the templatization, someone could look for the exact
behaviour, only with a different back-end. So we still need review and approval
for these nitpicks in UI, before or after the templatization.
Attachment #131858 - Attachment is obsolete: true
Attachment #132087 - Flags: review?(kiko)
sts (soft tab stop) is what you're looking for.

set expandtab
set sts=4
set shiftwidth=4
Comment on attachment 132087 [details] [diff] [review]
Patch ver 4


I loaded this on my local version to test it.

>+++ editusers.cgi	24 Sep 2003 19:48:45 -0000
>-    print "To change ${user}'s permissions, go back and <a href=\"editusers.cgi?action=edit&user=" . url_quote($user)."\">edit this user</A>";
>+    print "To change ${user}'s permissions, go back and " .
>+        "<a href=\"editusers.cgi?action=edit&user=" . url_quote($user) .
>+        "\">edit</a> this user";

A period is missing from the end of this phrase.

>Index: editkeywords.cgi

The default PutTrailer() handler here outputs a default link back to the query
page instead of the index. I don't advocate you change this here, but please
add an XXX: comment in the function indicating it's inconsistent with other
PutTrailer() implementations (some of which link to the index page, some of
which link to the query page).

>Index: editcomponents.cgi

Note that editcomponents is also inconsistent in linking to query :-(

>Index: editgroups.cgi
>+      print "<a href=\"editgroups.cgi?action=del&group=$gid\">" .
>+            "View the list of which records are affected</a><br>";

A period is missing here too.

I've gone over the rest, and it's okay, so this should be the last go.
Attachment #132087 - Flags: review?(kiko) → review-
Attached patch Patch ver 5Splinter Review
Thanks for the suggestions! Should be fixed.

Since I didn't want to hyperlink a dot, I made hyperlinking in the "View the
list of which records are affected." sentence consistent with the "<a
...>edit</a> this user" thing.
Attachment #132087 - Attachment is obsolete: true
Attachment #132205 - Flags: review?(kiko)
<-- me
Assignee: mattyt-bugzilla → jocuri
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Comment on attachment 132205 [details] [diff] [review]
Patch ver 5

if you're going to be checking that code in, add an 'XXX' before the comment
text you added, so it reads:

XXX: This implementation ...

that's the standard way of saying "this should be  better".
Attachment #132205 - Flags: review?(kiko) → review+
Flags: approval?
Flags: approval? → approval+
Vladd, if your CVS account isn't set up yet, let me know and I can check this in
for you.
>> I can check this in for you.

That would be great :-) thanks!
>> I can check this in for you.

That would be great :-) thanks!
I figured since you insisted I should do it sooner than later..

/cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v  <--  editcomponents.cgi
new revision: 1.33; previous revision: 1.32
/cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v  <--  editgroups.cgi
new revision: 1.28; previous revision: 1.27
/cvsroot/mozilla/webtools/bugzilla/editkeywords.cgi,v  <--  editkeywords.cgi
new revision: 1.15; previous revision: 1.14
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.39; previous revision: 1.38
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.47; previous revision: 1.46

FIXED. Sorry for all the harassment :-)
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
>> since you insisted

The first time I forgot to put you on the CC list ;-)
QA Contact: jake → default-qa
You need to log in before you can comment on or make changes to this bug.