Last Comment Bug 80169 - JavaScript-enhanced keyword editing
: JavaScript-enhanced keyword editing
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Creating/Changing Bugs (show other bugs)
: 2.13
: All All
: P3 enhancement with 3 votes (vote)
: Bugzilla 3.2
Assigned To: Teemu Mannermaa (:wicked)
: default-qa
:
Mentors:
: 80168 102852 140920 330017 377158 (view as bug list)
Depends on:
Blocks: 452734
  Show dependency treegraph
 
Reported: 2001-05-10 18:06 PDT by Myk Melez [:myk] [@mykmelez]
Modified: 2011-08-05 21:10 PDT (History)
20 users (show)
LpSolit: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
sample implementation (4.00 KB, patch)
2001-05-10 18:24 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
updated patch (9.36 KB, patch)
2001-05-29 12:22 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
version that works on both NS4.x and Mozilla (9.67 KB, patch)
2001-06-11 14:12 PDT, Myk Melez [:myk] [@mykmelez]
no flags Details | Diff | Splinter Review
uses commas as separators (9.39 KB, patch)
2001-06-14 18:40 PDT, Myk Melez [:myk] [@mykmelez]
jouni: review-
Details | Diff | Splinter Review
JavaScript functions to move keywords around (4.90 KB, application/octet-stream)
2003-04-03 14:01 PST, Jon Wilmoth
no flags Details
Patch to add keywords to bug.choices variable (4.02 KB, patch)
2003-04-03 14:03 PST, Jon Wilmoth
no flags Details | Diff | Splinter Review
Patches to enhance keywords UI (12.08 KB, application/zip)
2003-04-08 17:03 PDT, Jon Wilmoth
myk: review-
Details
diffs for Bug.pm, enter_bug.cgi, create.html.tmpl, edit.html.tmpl, header.html.tmpl (7.52 KB, patch)
2003-05-07 14:12 PDT, Michael Morgan [:morgamic]
myk: review-
Details | Diff | Splinter Review
select.js used with attachment 122700 (above) put in js/ (4.76 KB, application/octet-stream)
2003-05-07 14:15 PDT, Michael Morgan [:morgamic]
myk: review-
Details
Patch to attachment select.js #122701 adds alpha sorting (785 bytes, patch)
2004-01-20 12:56 PST, Ian Bothwell
no flags Details | Diff | Splinter Review
The main keyword chooser file. (5.46 KB, text/plain)
2004-06-23 14:31 PDT, Anima Gupta
no flags Details
The keyword chooser template file (3.06 KB, text/plain)
2004-06-23 14:35 PDT, Anima Gupta
no flags Details
(deleted) (32 bytes, text/plain)
2004-06-23 14:44 PDT, Anima Gupta
no flags Details
Main changes needed to apply the keyword chooser on create.html.tmpl (2.28 KB, text/plain)
2004-06-23 15:11 PDT, Anima Gupta
no flags Details
Complete patch, not quite finished (15.99 KB, patch)
2007-04-13 00:30 PDT, Dave Miller [:justdave] (justdave@bugzilla.org)
no flags Details | Diff | Splinter Review
Complete patch, V1 (26.24 KB, patch)
2007-04-14 10:49 PDT, Teemu Mannermaa (:wicked)
justdave: review+
Details | Diff | Splinter Review
Complete patch, V1.1 (26.62 KB, patch)
2007-05-14 10:37 PDT, Teemu Mannermaa (:wicked)
wicked: review+
LpSolit: review+
Details | Diff | Splinter Review

Description Myk Melez [:myk] [@mykmelez] 2001-05-10 18:06:27 PDT
It should be possible to use JavaScript to significantly improve the editing of
the keywords field (while maintaining backwards compatibility for users without
JavaScript).

Among other solutions to this problem, it is possible to use JavaScript to
create two drop-down menus next to the keywords field.  The first menu contains
a list of keywords currently in the field, while the second menu contains a list
of all possible keywords.

When the user selects a keyword from the first ("remove") menu, that keyword is
removed from the field.  When the user selects a keyword from the second ("add")
menu, that keyword is added to the field.  When the user edits the keyword field
manually, those edits are reflected in the "remove" menu, so that menu and the
field are always in sync.

Future enhancements to this feature could include:

1. Displaying tool-tip descriptions of keywords as the user mouses over them in
the menus or the field;
2. Preventing a user from manually typing in an invalid keyword or from entering
a keyword twice (either manually or via the "add" menu); and 
3. Reducing UI clutter by only having an "add" menu (since removing a keyword is
arguably easy to accomplish manually, at least for those bugs whose list of
keywords is not longer than the length of the field).

But these enhancements are icing on the cake.  Even without them this feature
would make it significantly easier to edit the keywords field.
Comment 1 Myk Melez [:myk] [@mykmelez] 2001-05-10 18:22:11 PDT
*** Bug 80168 has been marked as a duplicate of this bug. ***
Comment 2 Myk Melez [:myk] [@mykmelez] 2001-05-10 18:24:25 PDT
Created attachment 34023 [details] [diff] [review]
sample implementation
Comment 3 Andreas Franke (gone) 2001-05-11 05:51:38 PDT
I was not convinced by your description, but I tried the patch and was
pleasantly surprised. It really makes sense, if the keyword list is small
enough. However, I don't think this solution will scale to the number of
keywords in use at mozilla.org . Keyword groups, or even hierarchical keyword
trees, should be implemented either before or together with this, so that the
system can have sub-menues. (Maybe even sub-sub-menues, but this could trigger
other usability problems if the menu hierarchy gets too complex.)
Without keyword groups, it is probably still faster to simply type the keyword
you want to add, than to search for it in an overly long list.

Minor nit: There could be better support for the "," separator used in the
keyword textbox. Right now, the comma appears in the "Remove" menu, and the
changes performed by the "Add" button don't do not generate commas, so that the
field ends up as mixed style.

Another small nit: You are able to add a keyword twice, and if you remove it,
the keyword still occurs in the "Remove" menu.

What about long keywords? Don't they make the "Remove" and "Add" buttons too
wide?
Comment 4 Myk Melez [:myk] [@mykmelez] 2001-05-29 12:22:24 PDT
Created attachment 36375 [details] [diff] [review]
updated patch
Comment 5 Myk Melez [:myk] [@mykmelez] 2001-05-29 12:23:43 PDT
The recently attached patch fixes the comma problem, prevents keywords from
being added twice, and is more robust and documented.
Comment 6 Andreas Franke (gone) 2001-05-30 01:51:06 PDT
The alert "That keyword is already in the keywords field!" is displayed only if
I try to add it twice in a row. If I add a keyword that is already in the
keywords field but which is not the first one, then the alert is not displayed.
Instead, the original instance is deleted from the list, and it is inserted as
the first keyword, so altogether it appears that it is moved to the beginning of
the list. I'd prefer it if the dialog would always be thrown when you add a
keyword that is already in the list.

Sometimes removing a keyword doesn't work. It disappears from the "Remove" menu
but the keywords field does not change. Currently, this problem occurs with all
keywords but the first one. This problem and the previous one may have the same
root cause.
Comment 7 Myk Melez [:myk] [@mykmelez] 2001-06-11 14:12:23 PDT
Created attachment 37943 [details] [diff] [review]
version that works on both NS4.x and Mozilla
Comment 8 Myk Melez [:myk] [@mykmelez] 2001-06-11 14:13:44 PDT
The new attachment fixes the problems Andreas reported by working around a
problem NS4.x had with the regular expression that finds keywords in the keyword
field.

Comment 9 Andreas Franke (gone) 2001-06-14 13:56:17 PDT
Does this patch depend on other patches that haven't been checked in yet?
When I apply the latest patch to bug_form.pl and update to the CVS tip,
I get the following software error when trying to view a bug:

select bit, description, (bit & 128 != 0) from groups where bit & 1016 != 0 and
isbuggroup != 0 and
(isactive = 1 or (bit & 128 != 0)) order by bit: Unknown column 'isactive' in
'where clause' at globals.pl
line 195. 
Comment 10 Andreas Franke (gone) 2001-06-14 14:11:24 PDT
Never mind. I ran checksetup.pl and it added that column for me.

The only remaining issue with this patch is a minor one:
When there are no keywords and you add more than one, then they are separated by
a single whitespace (" "), but without a comma. The default separator is a comma
followed by a whitspace (", "), and it would be nice if this separator could be
used even in this case. Otherwise, it's great. :-)
Comment 11 Myk Melez [:myk] [@mykmelez] 2001-06-14 18:40:43 PDT
Created attachment 38530 [details] [diff] [review]
uses commas as separators
Comment 12 Andreas Franke (gone) 2001-06-15 02:12:29 PDT
Works fine now. t(ested)=afranke on Netscape 4.76 / Solaris.

It would be nice to see this in the next release.
Comment 13 Gervase Markham [:gerv] 2001-10-14 18:16:58 PDT
That's a whacking great chunk of JS to add to an already-bloated page. Is the
added function really worth it?

Gerv
Comment 14 Matthew Tuck [:CodeMachine] 2001-10-21 07:31:27 PDT
If it was external JS wouldn't there be cache improvements?  The file should
rarely never change.
Comment 15 Gervase Markham [:gerv] 2001-11-12 15:09:29 PST
We would reduce the bulk significantly by changing the comments to template
comments after templatisation.

Gerv
Comment 16 Dave Miller [:justdave] (justdave@bugzilla.org) 2001-11-17 18:12:38 PST
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 17 Jouni Heikniemi 2002-05-30 05:45:22 PDT
Comment on attachment 38530 [details] [diff] [review]
uses commas as separators

This needs-work because it's pre-templatization stuff. But the feature itself
sounds good, some optional assistance with keywords is certainly necessary.
Comment 18 Jon Wilmoth 2003-04-03 14:01:30 PST
Created attachment 119349 [details]
JavaScript functions to move keywords around

This new file would be placed in the new "js" subdirectory of a bugzilla
install.
Comment 19 Jon Wilmoth 2003-04-03 14:03:32 PST
Created attachment 119350 [details] [diff] [review]
Patch to add keywords to bug.choices variable

Adds keyword array to bug.choices variable.
Comment 20 Jon Wilmoth 2003-04-03 14:22:41 PST
I've posted a couple of attachments (attachment ) as a proposed solution for 
this.  Basically this change includes three things:

1) Add list of all available keywords to bug.choices variable exposed to 
template.
2) Add js file containing all functions
3) Replace the [% IF bug.use_keywords %] block surrounding the keywords text 
box with the code below.

I couldn't get cvs to respond so I could create a patch file, here's the 
changes to template/en/default/edit.html.tmpl:

Replace:
<pre>
  [% IF bug.use_keywords %]
    <tr>
      <td align="right">
        <b>
          <a href="describekeywords.cgi"><u>K</u>eywords</a>:
        </b>
      <td colspan="5">
        <input name="keywords" accesskey="k"
               value="[% bug.keywords.join(', ') FILTER html %]" size="60">
      </td>
    </tr>
[% END %]
</pre>
with:
<pre>
  [% IF bug.use_keywords %]
    <tr>
      <td align="right">
        <b>
          <a href="describekeywords.cgi"><u>K</u>eywords</a>:
        </b>
      <td colspan="5">
        <input type="hidden" name="keywords" value="[% bug.keywords.join(', ') 
FILTER html %]" />
        <table width="100%" border="0" cellspacing="1" cellpadding="0">
          <tr>
            <td width="25%"><label title="List of remaing keywords available 
to describe this issue"><font size="-1"><b>Available</b></font></label></td>
            <td width="12%">&nbsp;</td>
            <td width="63%"><label title="List of keywords that apply to this 
issue"><font size="-1"><b>Selected</b></font></label></td>
          </tr>
          <tr>
            <td width="25%">
	        <select name="availableKeywords" multiple size="6">
				[% FOREACH keyword = 
bug.choices.legal_keywords %]
                	[% IF !bug.keywords.join(', ').match(keyword) %]
                		<option value="[% keyword FILTER html %]">[% 
keyword FILTER html %]</option>
                	[% END %]
                [% END %]
	        </select>
            </td>
            <td width="12%">
              <input type="button" name="Submit" value="&gt;&gt;"
			  	onclick="updateKeywords
(this.form.name, 'availableKeywords', 
this.form.name, 'selectedKeywords', 'keywords', 'selectedKeywords')"/>
			  <br/>
			  <br/>
              <input type="button" name="Submit2" value="&lt;&lt;"
			  	onclick="updateKeywords
(this.form.name, 'selectedKeywords', 
this.form.name, 'availableKeywords', 'keywords', 'selectedKeywords')"/>
            </td>
            <td width="63%">
              <select name="selectedKeywords" MULTIPLE size="6" 
onchange="concatKeywords('changeform', 'keywords', 'selectedKeywords');">
				[% FOREACH keyword = 
bug.choices.legal_keywords %]
                	[% IF bug.keywords.join(', ').match(keyword) %]
                		<option value="[% keyword FILTER html %]">[% 
keyword FILTER html %]</option>
                	[% END %]
                [% END %]
              </select>
            </td>
          </tr>
        </table>
      </td>
    </tr>
  [% END %]
</pre>
Comment 21 Jon Wilmoth 2003-04-03 14:25:50 PST
Oops also need to make reference to the js file in edit.html.tmpl.  I placed 
<code>&lt;script type="text/javascript" language="JavaScript" 
src="js/select.js"&gt;&lt;/script&gt;</code> in my global header template, but 
I beleive you could stick that line anywhere in the bug/edit.html.tmpl.
Comment 22 Jon Wilmoth 2003-04-08 17:03:29 PDT
Created attachment 119891 [details]
Patches to enhance keywords UI

This is a zip of individual patch files as well as the js file all in one.  The
previous patches did not take into account keywords are used in creation of
bugs...now it does.
Comment 23 Jon Wilmoth 2003-04-08 17:05:05 PDT
Requesting review.
Comment 24 Jon Wilmoth 2003-04-08 17:12:04 PDT
Comment on attachment 119891 [details]
Patches to enhance keywords UI

Requesting review of patch
Comment 25 Dave Miller [:justdave] (justdave@bugzilla.org) 2003-04-08 23:44:51 PDT
getting it out of my queue until the patches are reviewed
Comment 26 Gervase Markham [:gerv] 2003-04-09 00:16:47 PDT
Jon: best to attach patches as cvs diff -uN - and, if you are attaching ZIPs,
mark them as "application/zip", rather than patch (which comes down as text/plain.)

Does your patch implement the function as specced in the initial report? If so,
I'm not convinced it's something we actually want. The show_bug page is complex
enough that any UI additions have to meet a fairly strong standard of
usefulness. In this case, we aren't even adding a new field or new function,
merely adding two new UI elements for controlling an existing field.

I haven't seen any bug reports from Bugzilla users complaining that the keywords
field is hard to manage. Has anyone else?

Gerv 
Comment 27 Myk Melez [:myk] [@mykmelez] 2003-05-07 12:18:14 PDT
Comment on attachment 119891 [details]
Patches to enhance keywords UI

These patches fail to apply on a tip install in multiple places.  Can you post
an updated patch?  As Gerv says, "cvs diff -uN <filenames>" is ideal, although
"cvs diff -u <filenames>" plus the JS file as a separate attachment is ok if
you don't have CVS write access.
Comment 28 Michael Morgan [:morgamic] 2003-05-07 13:03:29 PDT
i've found the added functionality provided by this patch to be very useful -- 
especially when dealing with large groups of keywords.

i've gotten positive responses from the techs who use our install, and they 
prefer the patched version to the non-patched version.

i have it currently working on a install derived from the cvs-tip, so i will 
look into creating a patch for you guys to possibly use/review/etc.

more to come...
Comment 29 Michael Morgan [:morgamic] 2003-05-07 14:12:47 PDT
Created attachment 122700 [details] [diff] [review]
diffs for Bug.pm, enter_bug.cgi, create.html.tmpl, edit.html.tmpl, header.html.tmpl

there are the diffs with cvs-tip.  tested it, seems to work.  test it yourself
to make sure.  select.js must be added separately.
Comment 30 Michael Morgan [:morgamic] 2003-05-07 14:15:10 PDT
Created attachment 122701 [details]
select.js used with attachment 122700 [details] [diff] [review] (above) put in js/

it differs slightly in size, so i'm attaching this one again.
Comment 31 Myk Melez [:myk] [@mykmelez] 2003-05-23 13:22:19 PDT
Comment on attachment 122700 [details] [diff] [review]
diffs for Bug.pm, enter_bug.cgi, create.html.tmpl, edit.html.tmpl, header.html.tmpl

Sorry this took me so long to review.  I have a few problems with the
functionality implemented by this patch (haven't looked at the code itself
yet).

First, it requires JavaScript and won't function without it, which at the
moment is not allowed for code going into the Bugzilla tree.  We'd have to
change our policy to get this in, which I think will happen eventually, but
perhaps only for optional features that can be turned off via a param in
editparams.cgi, so this feature needs to either work for people without
JavaScript or be optional (and drive a policy change).

Second, the UI takes up a lot of space, and in the crowded bug form, space is
at a premium.  I don't know what the right solution is here, but some attention
needs to be paid to how to minimize the space taken up by the UI.

Third, for sufficiently large keywords lists, the "available" multi-select menu
won't show enough keywords to be much more practical than a single-select menu
and will be more difficult to use: moving up and down will require separate
actions, selections will be able to scroll out of view, etc.  A single-select
menu would be a better choice.

Fourth, the "selected" multi-select may suffer from the same problem as the
"available" one if users select more keywords than can be displayed in the
list.  I can't help thinking that there could be a better way of displaying
selected keywords, f.e. using DOM calls to add them to a comma-separated list
along with tiny buttcons (button icons) for unselecting each one.
Comment 32 Myk Melez [:myk] [@mykmelez] 2003-05-23 13:23:54 PDT
Comment on attachment 122701 [details]
select.js used with attachment 122700 [details] [diff] [review] (above) put in js/

Marking in accordance with review of companion patch.
Comment 33 Christopher Aillon (sabbatical, not receiving bugmail) 2003-08-24 18:32:34 PDT
Taking.  Unawares of this bug, I implemented this very thing for a private
installation of Bugzilla.  I showed it to myk who seemed to like it, and it
addresses all of myk's concerns in comment 31.  I will try and backport it this
week.
Comment 34 Ian Bothwell 2004-01-09 11:49:07 PST
I tried this patch and it works fine as long as you are using netscape and/or
IE. It does not work under Conquer.
Comment 35 Ian Bothwell 2004-01-09 12:14:29 PST
Also Sorry to spam but this patch does not include a solution for editing
multiple bugs at once. It only provides the new UI for new bugs and editing a
single bug at a time.
Comment 36 Ian Bothwell 2004-01-20 12:56:26 PST
Created attachment 139511 [details] [diff] [review]
Patch to attachment select.js #122701 adds alpha sorting

I have added a sort funciton that helps to keep the select boxes easy to read.
I am using the select.js in production environment and it works really well.
Feedback has been excelent.
Comment 37 Rémi Zara 2004-05-31 05:51:04 PDT
What about implementing something like php.net's search page auto-completion (http://www.php.net/
search.php) ?
A javascript is built each time (but only when) the keyword list is modified; it is included in each page 
where the 
keywords field is. The UI remains the same as it is today, except that when Javascript is enabled, a pop-
up appears letting you complete what you began to type. 
Comment 38 Anima Gupta 2004-06-23 14:31:51 PDT
Created attachment 151556 [details]
The main keyword chooser file.

- The is the mail js file which handles the widgets on the keyword chooser.
Comment 39 Anima Gupta 2004-06-23 14:35:37 PDT
Created attachment 151557 [details]
The keyword chooser template file

This is the keyword chooser template used by the keyword chooser js file.
Comment 40 Anima Gupta 2004-06-23 14:44:38 PDT
Created attachment 151558 [details]
(deleted)

Since the files are changed so much that patch does not make sense - attaching
the entire create.html.tmpl file - so that the relevant code to keyword chooser
can be applied.
Comment 41 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-06-23 14:57:45 PDT
Chris: the attachments just made on this bug (I guess there's one or two coming
yet) are the code you did for AOL (referenced in comment 33) that they just
released to us (copyright has been reassigned to the Mozilla Foundation
according to the email I got on the subject relayed from AOL Legal).  Since it's
now legal to be touched, and you're the original author, it'd make perfect sense
for you to be the one to finish backporting it, if you have time for it that is. :)

Let us know :)  If there's any pieces missing that you explicitly need, let
Anima know what's missing so she can post it.
Comment 42 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-06-23 15:01:05 PDT
I deleted the attachment in comment 40...  There are things in it that the
letter from AOL Legal says had to be removed before it could be shared.
Comment 43 Anima Gupta 2004-06-23 15:11:19 PDT
Created attachment 151559 [details]
Main changes needed to apply the keyword chooser on create.html.tmpl
Comment 44 Dave Miller [:justdave] (justdave@bugzilla.org) 2004-09-18 17:50:19 PDT
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.
Comment 45 Frédéric Buclin 2005-11-17 07:10:05 PST
The trunk is now frozen to prepare Bugzilla 2.22. Only bug fixes are accepted, no enhancement bugs. As this bug has a pretty low activity (especially from the assignee), it's retargetted to ---. If you want to work on it and you think you can have it fixed for 2.24, please retarget it accordingly (to 2.24).
Comment 46 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-04-13 00:30:50 PDT
Created attachment 261447 [details] [diff] [review]
Complete patch, not quite finished

This patch incorporates all the materials contributed by AOL along with some additional cleanup and some additional missing pieces retrieved from AOL that hadn't gotten posted.  This *almost* works...  problems that remain:

1) in Firefox, the chooser shows up at the top of the window instead of attached to the keywords field
2) in IE, the chooser shows up almost in the correct spot, but the list boxes are empty.
Comment 47 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-04-13 00:34:34 PDT
I'd appreciate some additional eyes on this since I'm out of ideas.
Comment 48 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-04-13 00:45:25 PDT
hmm, I forgot to add the hooks for show_bug.
Comment 49 Teemu Mannermaa (:wicked) 2007-04-14 05:47:40 PDT
*** Bug 377158 has been marked as a duplicate of this bug. ***
Comment 50 Teemu Mannermaa (:wicked) 2007-04-14 05:50:19 PDT
*** Bug 330017 has been marked as a duplicate of this bug. ***
Comment 51 Teemu Mannermaa (:wicked) 2007-04-14 10:49:10 PDT
Created attachment 261561 [details] [diff] [review]
Complete patch, V1

You got bitten by same symptom as in bug 300743. You need to add px word after top and left values. Otherwise FF just logs in Error Console that it can't parse the value and ignores it.

Here's an updated patch that should be complete. I tested it on both IE7 and Firefox. It has following changes:

 1) Now positioning is cross browser compatible by using functions from js/util.js. This means this patch is now dependent on my patch in bug 182082.

 2) I have no idea what that KeywordChooserObserver and IE bleeding stuff is. I can't see any bleeding and code doesn't work because chooserIEBleedOpenObserver (and probably also chooserIEBleedCloseObserver) are not defined. I just commented it out for this version but it should be removed unless somebody can fix it and proove that we need it.

 3) Refactored the JS code in bug/create/create.html.tmpl into Keyword Chooser JS and template. This way the code doesn't need to be duplicated. I also removed duplicated and fixed buggy code so that now keywords are correctly loaded into the Keyword Chooser widget.

 4) Keyword Chooser support is now available in enter_bug.cgi (incl. working when cloning and using keywords= parameter), show_bug.cgi, bug_list.cgi (change multiple), process_bug.cgi (for the bug shown after changes) and post_bug.cgi (it shows the submitted bug).
Comment 52 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-04-14 14:04:05 PDT
Comment on attachment 261561 [details] [diff] [review]
Complete patch, V1

Wheee!  Now time to get to the nitty gritty. :)

(In reply to comment #51)
>  1) Now positioning is cross browser compatible by using functions from
> js/util.js. This means this patch is now dependent on my patch in bug 182082.

Cool, the select-box handling code a the bottom of the keyword-chooser should probably go in util.js also.  It came out of a general file equivalent to util.js in AOL's Bugzilla, and I just stuffed it at the bottom of keyword-chooser.js because nothing else was using it yet, and we didn't seem to have a util.js or equivalent yet.

>  2) I have no idea what that KeywordChooserObserver and IE bleeding stuff is.
> I can't see any bleeding and code doesn't work because
> chooserIEBleedOpenObserver (and probably also chooserIEBleedCloseObserver) are
> not defined. I just commented it out for this version but it should be removed 
> unless somebody can fix it and proove that we need it.

That's something for caillon to answer, but I suspect it was a hack for IE 5.5 support (which was our minimum required browser on the IE side when we were working on this for AOL).  There may also be another missing piece somewhere which defines those.  If they turn out to be important, I can ask and try to get it.  Seems to work okay without errors on IE6, but I don't have an IE5.5 to try.  Maybe we don't care about IE5.5 nowadays.

>  4) Keyword Chooser support is now available in enter_bug.cgi (incl. working
> when cloning and using keywords= parameter), show_bug.cgi, bug_list.cgi
> (change multiple), process_bug.cgi (for the bug shown after changes) and
> post_bug.cgi (it shows the submitted bug).

Coolness!  Thanks a bunch!

Only minor nit I have at this point is we probably need to change the colors.  The css on this still contains the color scheme from AOL's Bugzilla.  We should probably make the box the same blue we're using in the banner or something.

r+ with that change though (pending an answer on IEBleed from caillon -- I poked him on that via IM, too, but he's away right now)
Comment 53 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-05-04 14:15:58 PDT
from caillon:

----
okay, from what i remember (digging way back), there was an issue with the opacity of the thing where it would cause the autocomplete dropdown to go behind the input control. I think there's probably a piece or something missing...
but we can probably get by without it for now.
guessing IE isn't that often used to report bugs
----
Comment 54 Teemu Mannermaa (:wicked) 2007-05-14 10:37:23 PDT
Created attachment 264763 [details] [diff] [review]
Complete patch, V1.1

1) Moved (and documented as best I could) select handling functions to js/util.js as requested.
2) Killed off the IE bleeding control related code. We can file a new bug to fix this if it turns out there is a need for it after all. IE does have problems with bleeding as seen in bug 182082 for the help div control.
3) Changed keyword chooser border color to match title background.

Carrying over r+ from justdave.
Comment 55 Teemu Mannermaa (:wicked) 2007-05-14 10:38:03 PDT
Comment on attachment 264763 [details] [diff] [review]
Complete patch, V1.1

Doesn't adding to filterexceptions.pl require two reviews?
Comment 56 Frédéric Buclin 2007-05-14 10:43:51 PDT
Comment on attachment 264763 [details] [diff] [review]
Complete patch, V1.1

r=LpSolit for the filterexceptions.pl part.
Comment 57 Frédéric Buclin 2007-05-14 10:57:12 PDT
Checking in buglist.cgi;
/cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v  <--  buglist.cgi
new revision: 1.354; previous revision: 1.353
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.156; previous revision: 1.155
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.186; previous revision: 1.185
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.360; previous revision: 1.359
done
Checking in show_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/show_bug.cgi,v  <--  show_bug.cgi
new revision: 1.51; previous revision: 1.50
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/js/keyword-chooser.js,v
done
Checking in js/keyword-chooser.js;
/cvsroot/mozilla/webtools/bugzilla/js/keyword-chooser.js,v  <--  keyword-chooser.js
initial revision: 1.1
done
Checking in js/util.js;
/cvsroot/mozilla/webtools/bugzilla/js/util.js,v  <--  util.js
new revision: 1.2; previous revision: 1.1
done
Checking in skins/standard/global.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v  <--  global.css
new revision: 1.36; previous revision: 1.35
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.101; previous revision: 1.100
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.101; previous revision: 1.100
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/keyword-chooser.html.tmpl,v
done
Checking in template/en/default/bug/keyword-chooser.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/keyword-chooser.html.tmpl,v  <--  keyword-chooser.html.tmpl
initial revision: 1.1
done
Checking in template/en/default/bug/show.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.html.tmpl,v  <--  show.html.tmpl
new revision: 1.18; previous revision: 1.17
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.77; previous revision: 1.76
done
Checking in template/en/default/bug/create/created.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/created.html.tmpl,v  <--  created.html.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/bug/process/header.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/process/header.html.tmpl,v  <--  header.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.42; previous revision: 1.41
done
Checking in template/en/default/list/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.54; previous revision: 1.53
done
Comment 58 Frédéric Buclin 2007-10-02 09:47:34 PDT
*** Bug 102852 has been marked as a duplicate of this bug. ***
Comment 59 Frédéric Buclin 2007-10-02 09:53:26 PDT
*** Bug 140920 has been marked as a duplicate of this bug. ***
Comment 60 Max Kanat-Alexander 2008-06-29 16:48:30 PDT
Added to the Bugzilla 3.2 release notes in bug 432331.
Comment 61 Daniel Kabs, reporting bugs since 2002 2008-10-20 06:26:04 PDT
Why am I not seeing this in 3.2-branch on landfill?

Example:

  https://landfill.bugzilla.org/bugzilla-3.2-branch/show_bug.cgi?id=123

No "very nice JavaScript popup" appearing.
Comment 62 Teemu Mannermaa (:wicked) 2008-10-20 07:17:31 PDT
(In reply to comment #61)
> Why am I not seeing this in 3.2-branch on landfill?

Because it was removed in bug 452734 due to it receiving too much bad user feedback.

Note You need to log in before you can comment on or make changes to this bug.