Closed
Bug 518293
Opened 15 years ago
Closed 14 years ago
Improve and Simplify the QuickSearch Documentation
Categories
(Bugzilla :: Query/Bug List, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.6
People
(Reporter: mkanat, Assigned: mkanat)
References
Details
Attachments
(2 files, 4 obsolete files)
334.08 KB,
image/png
|
Details | |
29.92 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
||
Assignee: query-and-buglist → mkanat
Status: NEW → ASSIGNED
Comment 2•15 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•15 years ago
|
||
I'm going to base the documentation on how QS will work after bug 518459.
Depends on: 518459
Assignee | ||
Comment 4•15 years ago
|
||
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•15 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•15 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•15 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.
Comment 8•15 years ago
|
||
> 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•15 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•15 years ago
|
Attachment #402475 -
Flags: review?(dkl) → review?(LpSolit)
Assignee | ||
Comment 10•15 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•14 years ago
|
Attachment #402475 -
Flags: review?(colin.ogilvie)
Attachment #402475 -
Flags: review?(LpSolit)
Attachment #402475 -
Flags: review-
Comment 11•14 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•14 years ago
|
Whiteboard: [needs new patch]
Assignee | ||
Comment 12•14 years ago
|
||
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•14 years ago
|
Whiteboard: [needs new patch]
Assignee | ||
Comment 13•14 years ago
|
||
Here's a screenshot of the full page for anybody who wants to read the docs without applying the patch.
Assignee | ||
Comment 14•14 years ago
|
||
Noticed that &emdash; doesn't work--it's —.
Attachment #420427 -
Attachment is obsolete: true
Attachment #420436 -
Flags: review?(LpSolit)
Attachment #420427 -
Flags: review?(LpSolit)
Comment 15•14 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—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> >+ [% ", " 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•14 years ago
|
Whiteboard: [new patch needed]
Assignee | ||
Comment 16•14 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> > >+ [% ", " 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•14 years ago
|
||
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•14 years ago
|
||
Comment on attachment 425669 [details] [diff] [review] v4 (contains rename) r=LpSolit
Attachment #425669 -
Flags: review?(LpSolit) → review+
Updated•14 years ago
|
Flags: approval3.6+
Flags: approval+
Assignee | ||
Comment 19•14 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
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [new patch needed]
You need to log in
before you can comment on or make changes to this bug.
Description
•