252739 - String magicness in editflagtypes.cgi prevents localization of admin/flag-type/edit.html.tmpl

Status: RESOLVED FIXED

(Bugzilla :: Administration, task)

Description

[Marc Schumann [:Wurblzap]]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0)
Build Identifier: 

The text (<input>-value) on the Include/Exclude/Remove Inclusion/Remove 
Exclusion buttons is used in editflagtypes.cgi to find out what button has been 
pressed. This prevents localization.

Reproducible: Always
Steps to Reproduce:
1. Localize template/en/admin/flag-type/edit.html.tmpl to language xy and put
   it in template/xy/default/admin/flag-type
2. Allow xy as a language in Bugzilla
3. Set your browser to prefer language xy
4. Try to use the buttons in editflagtypes.cgi
   (short of "Create", which works because there is no magic involved)
Actual Results:  
The buttons don't work because editflagtypes.cgi ignores them.

Expected Results:  
They should work no matter what text is on them.

Comment 1

[Dave Miller [:justdave]]

bah.

Yes, the proper way to do this is to make a hyphenated "name" attribute for the
button, with the actual name in front of the hyphen, and the "magic value" after
the hyphen, as part of the name.  Then the button text can be localized.  On the
back end, we just do a regexp match on the static part of the name in the params
to locate it and figure out which command by the second have of the field name.

We have existing code that does this on the charts pages, I believe.

Comment 2

[Marc Schumann [:Wurblzap]]

Attachment: The proper way :-)

Comment 3

[Christian Reis]

Comment on attachment 154638 [details] [diff] [review]
The proper way :-)

patched/editflagtypes.cgi	2004-07-29 >+	 $categoryActions[0] =~
s/^categoryAction-//

Maybe this would be more readable using an m// parenthesizing what came after
the hyphen, but it's not important.

I like patches that remove FORM callsites. I haven't tested this, but the code
reads correctly.

Comment 4

[Christian Reis]

If this is 2.18 material we might want a second reviewer who can actually test
this -- or the applier can please do so.

Comment 5

[Dave Miller [:justdave]]

Code has to work to go on the trunk, too.  If you're not sure, go ahead and
request the second review :)

Comment 6

[Marc Schumann [:Wurblzap]]

Comment on attachment 154638 [details] [diff] [review]
The proper way :-)

Asking for a second review as per comment 5...

Comment 7

[Marc Schumann [:Wurblzap]]

Comment on attachment 154638 [details] [diff] [review]
The proper way :-)

Asking for a second review as per comment 5...

(Got it wrong last time)

Comment 8

[Dave Miller [:justdave]]

*** Bug 260809 has been marked as a duplicate of this bug. ***

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 154638 [details] [diff] [review]
The proper way :-)

This looks like a good thing to do.  Ultimately we'll want to move to <button>,
which distinguishes between the field's name/value and its label, but this
solution works well for now. r=myk

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Seems like this would be useful to get into 2.18 in addition to the trunk for
the sake of our localization story, although it's not a blocker.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Oh, darn, this patch conflicts with a patch I'm working on, and the author is on
vacation.  Ok, checking in for author and taking responsibility for tree after
checkin.  Checked into both trunk and 2.18 branch:

Checking in editflagtypes.cgi;
/cvsroot/mozilla/webtools/bugzilla/editflagtypes.cgi,v  <--  editflagtypes.cgi
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/admin/flag-type/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/flag-type/edit.html.tmpl,v
 <--  edit.html.tmpl
new revision: 1.7; previous revision: 1.6
done
Checking in ../bz218/editflagtypes.cgi;
/cvsroot/mozilla/webtools/bugzilla/editflagtypes.cgi,v  <--  editflagtypes.cgi
new revision: 1.7.2.2; previous revision: 1.7.2.1
done
Checking in ../bz218/template/en/default/admin/flag-type/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/flag-type/edit.html.tmpl,v
 <--  edit.html.tmpl
new revision: 1.6.2.1; previous revision: 1.6
done