If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Improve and Simplify the QuickSearch Documentation

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
Query/Bug List
--
enhancement
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

Bugzilla 3.6
Dependency tree / graph
Bug Flags:
approval +
approval3.6 +
blocking3.6 +

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Assignee)

Description

8 years ago
After bug 518024, the quicksearch docs will need to be changed, to incorporate the fact that it now accepts all Bugzilla fields. Also, it could use some general modernization and simplification.
(Assignee)

Comment 1

8 years ago
Created attachment 402297 [details] [diff] [review]
Work In Progress
Assignee: query-and-buglist → mkanat
Status: NEW → ASSIGNED

Comment 2

8 years ago
Does it make sense to add quicksearch_field_names in Template.pm when you only need it in the documentation? You could copy this in page.cgi.
(Assignee)

Comment 3

8 years ago
I'm going to base the documentation on how QS will work after bug 518459.
Depends on: 518459
(Assignee)

Comment 4

8 years ago
Created attachment 402475 [details] [diff] [review]
v1

Okay, this is a complete re-write of the QuickSearch docs--hopefully one that should be sensible and accessible to all Bugzilla users, not just the advanced ones! :-) I'd like to see more people using QuickSearch, I think, particularly now that we've worked on simplifying it and since we're going to integrate it into fulltext search.

I also changed the copyright and contributor statement in the file, not to discredit anybody (in fact, I really don't like doing that), but because there was no Initial Developer statement in the file, and "N.N.", the first listed contributor, I can't figure out who that would be in order to credit them with an original copyright. Since I re-wrote the entire contents of the file and didn't retain any of the old content (other than the quicksearch form, which is identical to the quicksearch form elsewhere in the code) I just took the copyright and Initial Developer credit, which solves the whole problem.
Attachment #402297 - Attachment is obsolete: true
Attachment #402475 - Flags: review?(dkl)
Attachment #402475 - Flags: review?(colin.ogilvie)

Comment 5

8 years ago
(In reply to comment #4)
> I also changed the copyright and contributor statement in the file, not to
> discredit anybody (in fact, I really don't like doing that), but because there
> was no Initial Developer statement in the file, and "N.N.", the first listed
> contributor, I can't figure out who that would be in order to credit them with
> an original copyright. Since I re-wrote the entire contents of the file and
> didn't retain any of the old content (other than the quicksearch form, which is
> identical to the quicksearch form elsewhere in the code) I just took the
> copyright and Initial Developer credit, which solves the whole problem.

You should still list wurblzap as a contributor. Completely altering a file doesn't give you the right to remove him from the contributor list, AFAIK. And probably he will know who N.N. is.
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> You should still list wurblzap as a contributor. 

  I'm not sure that anything he contributed remains, but I'm willing to list him if he's responsible for the quicksearch form, which I suspect he is.

> Completely altering a file
> doesn't give you the right to remove him from the contributor list, AFAIK.

  Mmm, if the file is entirely my original content (which it is, theoretically, except for the quicksearch form) then actually I can, if I want to. But I'd be happy to put Wurblzap back if there is code in the quicksearch form that's his, which in fact I suspect there is. I can do it on checkin, if it passes review.
(Assignee)

Comment 7

8 years ago
In any case, the point of modifying the license block (an incredibly trivial part of this otherwise very large patch) was to find a way to comply with the MPL, and since I had the opportunity to do so, I did. I really am fine with adding anybody back to the Contributor list who contributed any significant amount of code that remains in the file.
> except for the quicksearch form) then actually I can, if I want to. But I'd be
> happy to put Wurblzap back if there is code in the quicksearch form that's his,
> which in fact I suspect there is. I can do it on checkin, if it passes review.

There is no significant original code of mine in there; in bug 70907, I simply ported Bugzilla's old static quicksearch.html to template/en/default/pages/quicksearch.html.tmpl, replaced the js stuff by the necessities to do it server-side, and I did some minor markup cleanup. Same for quicksearchhack. As far as I can see, all that's left is <form name="f" action="buglist.cgi" method="get">, which I don't really cling to.

Oh, and I don't think you should drop the “Load Search Form” functionality.

Comment 9

8 years ago
Comment on attachment 402475 [details] [diff] [review]
v1

Only thing that jumped out at me from reading the patch was a bit I'd reword...

>Index: template/en/default/pages/quicksearch.html.tmpl
>+  <li>[% terms.Bugzilla %] searches not just for the exact word you put in,
>+    but also for any word that <strong>contains</strong> the word you put in.

Bugzilla does not just search for the exact word that you put in, but also for any word that contains that word.

>+    So, for example, searching for "cat" would also find [% terms.bugs %]
>+    that contain "<strong>cat</strong>ch" and 
>+    "certifi<strong>cat</strong>e".</li>

So, for example, searching for "cat" would also find bugs that contain it - for example, a bug mentioning "<strong>cat</strong>ch" or "certifi<strong>cat</strong>e".

I'll take a quick look later at it in HTML format later.
(Assignee)

Updated

8 years ago
Attachment #402475 - Flags: review?(dkl) → review?(LpSolit)
(Assignee)

Updated

8 years ago
Blocks: 527641
(Assignee)

Updated

8 years ago
Blocks: 448609
(Assignee)

Updated

8 years ago
Blocks: 529201
(Assignee)

Comment 10

8 years ago
This is now a blocker since the stuff that changed QuickSearch was checked in, but these docs that explain the new system haven't been.
Flags: blocking3.6+

Updated

8 years ago
Attachment #402475 - Flags: review?(colin.ogilvie)
Attachment #402475 - Flags: review?(LpSolit)
Attachment #402475 - Flags: review-

Comment 11

8 years ago
Comment on attachment 402475 [details] [diff] [review]
v1

bitrotten in page.cgi (the hook name changed) and user-error.html.tmpl (doesn't match what landed in the blocker)

Updated

8 years ago
Whiteboard: [needs new patch]
(Assignee)

Comment 12

8 years ago
Created attachment 420427 [details] [diff] [review]
v2

Okay, this fixes the bitrot and also modifies the language as Colin suggested.

I also added [?] link next to the header/footer quicksearch, that links to this page.
Attachment #402475 - Attachment is obsolete: true
Attachment #420427 - Flags: review?(LpSolit)
(Assignee)

Updated

8 years ago
Whiteboard: [needs new patch]
(Assignee)

Comment 13

8 years ago
Created attachment 420434 [details]
Full-Page Screenshot

Here's a screenshot of the full page for anybody who wants to read the docs without applying the patch.
(Assignee)

Comment 14

8 years ago
Created attachment 420436 [details] [diff] [review]
v3

Noticed that &emdash; doesn't work--it's &mdash;.
Attachment #420427 - Attachment is obsolete: true
Attachment #420436 - Flags: review?(LpSolit)
Attachment #420427 - Flags: review?(LpSolit)

Comment 15

8 years ago
Comment on attachment 420436 [details] [diff] [review]
v3

>Index: skins/contrib/Dusk/.cvsignore

>+page.css
> params.css
> release-notes.css

Shouldn't this last item be removed now that release-notes.css no longer exists? Also, shouldn't you remove release-notes.css from contrib/Dusk/ and custom/ as they are now irrelevant?



>Index: template/en/default/pages/quicksearch.html.tmpl

This template doesn't pass tests 8 and 9:

#   Failed test '(en/default) template/en/default/pages/quicksearch.html.tmpl has unfiltered directives:                                                                            
#   43: field_descs.product
#   44:+ field_descs.component
#   45:+ field_descs.keywords
#   46:+ field_descs.short_desc
#   47: field_descs.status_whiteboard
#   48: field_descs.longdesc
#   71: field_descs.longdesc
#   105: first_field.name
#   106: first_field.name.replace('^cf_')
# --ERROR'
#   at t/008filter.t line 132.


not ok 98 - template/en/default/pages/quicksearch.html.tmpl contains invalid bare words (e.g. 'bug') --WARNING
224:  with the highest priority.
        "P2" means the second-highest priority, and so on.
        <p>Searching for "<strong>P1-P3</strong>" will find bugs in any of the
          three highest priorities, and so on.</p>
#   at t/009bugwords.t line 90.



>+<ul class="qs_help">
>+  <li>If you just put a word or series of words in the search box, 
>+    [%+ terms.Bugzilla %] will search the [% field_descs.product %],
>+    [%+ field_descs.component %], 
>+    [%+ IF use_keywords %][%+ field_descs.keywords %],[% END %]
>+    [%+ field_descs.short_desc %], 
>+    [%+ IF Param('usestatuswhiteboard') %][% field_descs.status_whiteboard %],[% END %]
>+    and [% field_descs.longdesc %] fields for your word or words.</li>

It always looks for the status whiteboard, independently of the 'usestatuswhiteboard' parameter. Also, all these fields must be HTML filtered, see above.


>+  <li>Typing just a <strong>number</strong> in the search box will take
>+    you directly to the [% terms.bug %] with that id. 

Nit: I prefer to read id uppercase. Being a non-native english speaker, I not immediately think ID, but 'it' with a typo. :)


>+      Also, just typing the <strong>alias</strong> of [% terms.abug %]
>+      will take you to do that [% terms.bug %].

s/do// ? Or I don't understand the meaning of this sentence. :)


>+    "<strong>cat</strong>ch" or "certifi<strong>cat</strong>e". It will
>+    not find partial words in the [% field_descs.longdesc %] field 
>+    though&mdash;only full words are matched, there.</li>

Same for keywords, unfortunately.


>+[% USE Bugzilla %]
>+[% IF Bugzilla.active_custom_fields.size %]

Move [% USE Bugzilla %] at the top of the file, so that we don't inadvertendly duplicate it if we later need another Bugzilla.foo earlier in the code and don't notice this one.


>+    <kbd>cf_</kbd>. So for example, <kbd>[% first_field.name %]</kbd> can be
>+    referred to as <kbd>[% first_field.name.replace('^cf_') %]</kbd>,

Must be HTML filtered too. Cool idea to take a real custom field as an example! :)


>+[% FOREACH field = quicksearch_field_names.keys %]
>+  [% description = field_descs.$field || bug_fields.${field}.description %]

field_descs.none.tmpl already does this for you, so field_descs.$field will always be defined.


>+  [% values = quicksearch_field_names.${field} %]
>+  [% field_table.$description = quicksearch_field_names.${field} %]

'values' is unused. I'm pretty sure this makes this FOREACH loop useless.


>+          [% FOREACH nickname = field_table.$desc %]
>+            <kbd>[% nickname FILTER html %]</kbd>
>+            [% ",&nbsp; " UNLESS loop.last %]
>+          [% END %]

Nit: could be done as <kbd>[% field_table.$desc.join(', ') FILTER html %]</kbd>.


>+        making <kbd>FIX</kbd> the first word of your search will find all
>+        [% terms.bugs %] with a resolution of <kbd>FIXED</kbd> .

Must be [%+ terms.bugs %]


>+      <td class="field_nickname">"<strong>P1</strong>" (as a word anywhere in
>+        the search) means "find [% terms.bugs %] with the highest priority. 

Really? When I type P1 alone, all bugs are returned and this string is ignored. Either the logic behind it has been removed, or there is a bug in the QS code.
Attachment #420436 - Flags: review?(LpSolit) → review-

Updated

8 years ago
Whiteboard: [new patch needed]
(Assignee)

Comment 16

8 years ago
(In reply to comment #15)
> Shouldn't this last item be removed now that release-notes.css no longer
> exists?

  Ah, yeah, done.

> Also, shouldn't you remove release-notes.css from contrib/Dusk/ and
> custom/ as they are now irrelevant?

  Mmm, they might have been modified, as unlikely as that seems.

> It always looks for the status whiteboard, independently of the
> 'usestatuswhiteboard' parameter.

  Yes, but if the parameter isn't on, there's a pretty good chance that users won't have any idea what that field is.

> 'values' is unused. I'm pretty sure this makes this FOREACH loop useless.

  Ah, yeah, "values" was unused, but the loop is still important so that we can create a data structure where we can sort by the description.

> >+          [% FOREACH nickname = field_table.$desc %]
> >+            <kbd>[% nickname FILTER html %]</kbd>
> >+            [% ",&nbsp; " UNLESS loop.last %]
> >+          [% END %]
> 
> Nit: could be done as <kbd>[% field_table.$desc.join(', ') FILTER html
> %]</kbd>.

  Ah, yeah, I didn't because I didn't want the commas in the monospace font.

> Really? When I type P1 alone, all bugs are returned and this string is ignored.
> Either the logic behind it has been removed, or there is a bug in the QS code.

  Ah, yes, that would be a bug; it should be filed separately.
(Assignee)

Comment 17

8 years ago
Created attachment 425669 [details] [diff] [review]
v4 (contains rename)

Here's a new patch with all comments addressed. I generated this with "bzr diff", so it contains a rename that won't be done automatically if you use "bzr patch" or simply "patch -p0". I renamed release-notes.css to page.css (so now you can see what I actually added, instead of just the whole file).
Attachment #420436 - Attachment is obsolete: true
Attachment #425669 - Flags: review?(LpSolit)

Comment 18

8 years ago
Comment on attachment 425669 [details] [diff] [review]
v4 (contains rename)

r=LpSolit
Attachment #425669 - Flags: review?(LpSolit) → review+

Updated

8 years ago
Flags: approval3.6+
Flags: approval+
(Assignee)

Comment 19

8 years ago
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/                       
modified .bzrignore
modified page.cgi
renamed skins/standard/release-notes.css => skins/standard/page.css
modified template/en/default/global/common-links.html.tmpl
modified template/en/default/global/user-error.html.tmpl
modified template/en/default/pages/quicksearch.html.tmpl
missing template/en/default/pages/quicksearchhack.html.tmpl
modified template/en/default/pages/quicksearchhack.html.tmpl
modified template/en/default/pages/release-notes.html.tmpl
Committed revision 6966.


Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/3.6/                         
modified .bzrignore
modified page.cgi
renamed skins/standard/release-notes.css => skins/standard/page.css
modified template/en/default/global/common-links.html.tmpl
modified template/en/default/global/user-error.html.tmpl
modified template/en/default/pages/quicksearch.html.tmpl
missing template/en/default/pages/quicksearchhack.html.tmpl
modified template/en/default/pages/quicksearchhack.html.tmpl
modified template/en/default/pages/release-notes.html.tmpl                     
Committed revision 6962.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Whiteboard: [new patch needed]

Updated

8 years ago
Blocks: 498295
(Assignee)

Updated

8 years ago
Keywords: relnote
(Assignee)

Comment 20

8 years ago
Added to the release notes in bug 547466.
Keywords: relnote
Duplicate of this bug: 107860
Duplicate of this bug: 454386
You need to log in before you can comment on or make changes to this bug.