1146782 - backport bug 1159589 to bmo (migrate autocomplete from yui to jquery)

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: User Interface, defect)

Description

[:glob ✱]

rewrite user matching in jquery, so we can drop yui completely from bug_modal.

Comment 1

[:glob ✱]

Attachment: 1146782_1.patch

- backport yui -> jquery from upstream
- convert prodCompSearch from jquery-ui to devBridgeAutocomplete
- convert bug-modal from jquery-ui to devBridgeAutocomplete
- changes User.match api param from maxusermatches to limit to match trunk & docs
  - maxusermatches never worked anyhow due to a taint error

Comment 2

[:glob ✱]

Attachment: 1146782_2.patch

- fix escaping issue

Comment 3

[David Lawrence [:dkl]]

Comment on attachment 8603920 [details] [diff] [review]
1146782_2.patch

Review of attachment 8603920 [details] [diff] [review]:
-----------------------------------------------------------------

When the value in the autocomplete list extends past the right side, there is no horizontal scroll bar to allow viewing the entire string. So a lot of components with the same prefix look the same and you cannot tell which to select. This is same in the product chooser (enter_bug.cgi, describecomponents.cgi) but not as bad as its text input is larger. It is really noticable in the search on show_bug.cgi. This did not happen with the old pre-devbridge autocomplete as it would stretch to accommodate the longest string.

Will keep reviewing the rest while you take a look at the above.

dkl

Comment 4

[:glob ✱]

Attachment: 1146782_3.patch

as per irc here's an updated patch that incorporates the suggestions width issue, as well as some other fixes around error management and styling.

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 8606848 [details] [diff] [review]
1146782_3.patch

Review of attachment 8606848 [details] [diff] [review]:
-----------------------------------------------------------------

fox on commit bit rot due to keyword disabling patch:

Hunk #1 FAILED at 235.
1 out of 1 hunk FAILED -- saving rejects to file template/en/default/bug/field.html.tmpl.rej

1. Multi select fields such as Mentors and CC do not add the comma/space after first choice. As a strange side effect, if I manually add a space after my first choice, the code removes the space automatically :) It only works if I manually enter a comma then a space.
2. Same as above with Keywords field. Drop down of choices for a second keyword only display if I manually enter comma and space.
3. The results list is same width as the input field which is fine in most places except for requests.cgi and query.cgi (Search by People). In those and possibly others, the list is too narrow and a lot of the user name is cut off. This is not a regression as this also happens with the old YUI method.
4. Comment tagging for the non-modal form is not working for me. When I click [tag] it scrolls me back to the top of the page and append '#' the URL. The tag input box is not displayed. Same for deleting a tag, except it does remove the tag from the page but does not update the database.

dkl

::: extensions/ProdCompSearch/web/js/prod_comp_search.js
@@ +117,5 @@
> +                },
> +                onSearchStart: function(params) {
> +                    var that = $(this);
> +                    params.match = $.trim(params.match);
> +                    that.addClass('autocomplete-running');

I see this is being added, but when I do searches, I do not see the throbber at all. Can you verify this is the case on your side?

::: template/en/default/bug/field.html.tmpl
@@ +240,5 @@
> +              name="[% field.name FILTER html %]"
> +              data-values="[% field.name FILTER html %]"
> +              value="[% value FILTER html %]">
> +       <script type="text/javascript">
> +          if (typeof BUGZILLA.autocomplete_values === 'undefined')

Shouldn't this be BUGZILLA.keyword_values or BUGZILLA.autocomplete_keyword_values as to be more descriptive?

::: template/en/default/search/field.html.tmpl
@@ +63,5 @@
> +           value="[% value FILTER html %]"
> +           data-values="[% field.name FILTER html %]">
> +    <script type="text/javascript">
> +      if (typeof BUGZILLA.autocomplete_values === 'undefined')
> +        BUGZILLA.autocomplete_values = [];

Similar here. Should we call this BUGZILLA.autocomplete_keyword_values or similar in case we have more than one autocomplete field in a page?

Comment 6

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #5)
> 1. Multi select fields such as Mentors and CC do not add the comma/space
> after first choice. As a strange side effect, if I manually add a space
> after my first choice, the code removes the space automatically :) It only
> works if I manually enter a comma then a space.

what's going on is the comma is a delimiter, not a space.  when you're hitting space it's actually performing a search, hitting the local search cache, and rendering the result trimmed.

i'll see what i can do to work around this... appending a space shouldn't trigger a search.

> 4. Comment tagging for the non-modal form is not working for me.

js error: YAHOO.util.Connect is undefined
looks like i'm removing one yui module too many.

> I see this is being added, but when I do searches, I do not see the throbber
> at all. Can you verify this is the case on your side?

the throbber works for me in all fields .. can you provide STR?  any js or http errors?

> > +          if (typeof BUGZILLA.autocomplete_values === 'undefined')
> 
> Shouldn't this be BUGZILLA.keyword_values or
> BUGZILLA.autocomplete_keyword_values as to be more descriptive?

the field name is the key in that variable:
> BUGZILLA.autocomplete_values['[% field.name FILTER js %]'] = [
so keywords are stored in autocomplete_values['keywords']

Comment 7

[:glob ✱]

Attachment: 1146782_4.patch

- adds missing yui modules for comment tagging
- spaces no longer trigger a new search
- ", " appended to field when suggestion is selected

Comment 8

[David Lawrence [:dkl]]

(In reply to Byron Jones ‹:glob› from comment #6)
> (In reply to David Lawrence [:dkl] from comment #5)
> > 1. Multi select fields such as Mentors and CC do not add the comma/space
> > after first choice. As a strange side effect, if I manually add a space
> > after my first choice, the code removes the space automatically :) It only
> > works if I manually enter a comma then a space.
> 
> what's going on is the comma is a delimiter, not a space.  when you're
> hitting space it's actually performing a search, hitting the local search
> cache, and rendering the result trimmed.
> 
> i'll see what i can do to work around this... appending a space shouldn't
> trigger a search.
> 
> > 4. Comment tagging for the non-modal form is not working for me.
> 
> js error: YAHOO.util.Connect is undefined
> looks like i'm removing one yui module too many.
> 
> > I see this is being added, but when I do searches, I do not see the throbber
> > at all. Can you verify this is the case on your side?
> 
> the throbber works for me in all fields .. can you provide STR?  any js or
> http errors?
> 
> > > +          if (typeof BUGZILLA.autocomplete_values === 'undefined')
> > 
> > Shouldn't this be BUGZILLA.keyword_values or
> > BUGZILLA.autocomplete_keyword_values as to be more descriptive?
> 
> the field name is the key in that variable:
> > BUGZILLA.autocomplete_values['[% field.name FILTER js %]'] = [
> so keywords are stored in autocomplete_values['keywords']

Fixed now, fixed now, somehow is working for now (not sure what was going on before), and ah, I see that now.

dkl

Comment 9

[David Lawrence [:dkl]]

Comment on attachment 8609972 [details] [diff] [review]
1146782_4.patch

Review of attachment 8609972 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

Comment 10

[:glob ✱]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   95e71ee..f8b9848  master -> master

filed upstream bug 1168439 for the "doesn't append a comma and eats a space" issue.