65316 - Typos on edit*.cgi.

Status: RESOLVED FIXED

(Bugzilla :: Administration, task, P4)

Description

[Matthew Tuck [:CodeMachine]]

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" ]

Comment 1

[Matthew Tuck [:CodeMachine]]

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.

Comment 2

[Dave Miller [:justdave]]

moving to real milestones...

Comment 3

[Matthew Tuck [:CodeMachine]]

Moving to new Bugzilla product ...

Comment 4

[Matthew Tuck [:CodeMachine]]

Taking ...

Comment 5

[Dave Miller [:justdave]]

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.

Comment 6

[Vlad Dascalu]

Attachment: Patch ver 1

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.

Comment 7

[Vlad Dascalu]

Attachment: Patch ver 2

I forgot about a capitalization fix in editcomponents.cgi.

Comment 8

[Christian Reis]

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?

Comment 9

[Vlad Dascalu]

Attachment: Patch ver 3

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.

Comment 10

[Christian Reis]

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 11

[Christian Reis]

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).

Comment 12

[Christian Reis]

Just clarifying (and now that I've done some grepping), I'm suggesting 

  s/main bugs page/index/

on its only occurence.

Comment 13

[Vlad Dascalu]

Attachment: Patch ver 4

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.

Comment 14

[Dave Miller [:justdave]]

sts (soft tab stop) is what you're looking for.

set expandtab
set sts=4
set shiftwidth=4

Comment 15

[Christian Reis]

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.

Comment 16

[Vlad Dascalu]

Attachment: Patch ver 5

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.

Comment 17

[Vlad Dascalu]

<-- me

Comment 18

[Christian Reis]

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".

Comment 19

[Christian Reis]

Vladd, if your CVS account isn't set up yet, let me know and I can check this in
for you.

Comment 20

[Vlad Dascalu]

>> I can check this in for you.

That would be great :-) thanks!

Comment 21

[Vlad Dascalu]

>> I can check this in for you.

That would be great :-) thanks!

Comment 22

[Christian Reis]

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 :-)

Comment 23

[Vlad Dascalu]

>> since you insisted

The first time I forgot to put you on the CC list ;-)