Last Comment Bug 877545 - quicksearch shouldn't treat apostrophes as quote characters
: quicksearch shouldn't treat apostrophes as quote characters
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: unspecified
: All All
: -- normal with 1 vote (vote)
: Bugzilla 4.4
Assigned To: Byron Jones ‹:glob›
: default-qa
Mentors:
Depends on:
Blocks: 918647
  Show dependency treegraph
 
Reported: 2013-05-29 21:33 PDT by Byron Jones ‹:glob›
Modified: 2013-09-19 21:10 PDT (History)
1 user (show)
glob: approval+
glob: approval4.4+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1 (2.08 KB, patch)
2013-05-29 21:45 PDT, Byron Jones ‹:glob›
LpSolit: review-
Details | Diff | Review
877545_2.patch (2.47 KB, patch)
2013-08-28 10:20 PDT, Byron Jones ‹:glob›
LpSolit: review-
Details | Diff | Review
877545_3.patch (2.97 KB, patch)
2013-08-28 22:33 PDT, Byron Jones ‹:glob›
mail: review+
Details | Diff | Review

Description Byron Jones ‹:glob› 2013-05-29 21:33:17 PDT
if you perform a quicksearch for a word with an apostrophe, you get the following error:

> Bugzilla is unable to parse your query correctly: can't.
> If you use quotes to enclose strings, make sure both quotes are present.
> If you really want to look for a quote in a string, type \" instead of ".
> For instance, "I'm so \"special\", really" (with quotes) will be interpreted
> as I'm so "special", really. 

this is counter-intuitive and painful.

" should be the only valid quote character.


this behaviour was changed by bugzilla 4.2 -- older versions don't treat ' as a quote character.
Comment 1 Byron Jones ‹:glob› 2013-05-29 21:45:06 PDT
Created attachment 755773 [details] [diff] [review]
patch v1
Comment 2 Byron Jones ‹:glob› 2013-05-29 21:47:26 PDT
>     # to sanely search for contradictions. As this behavour isn't

of course i mean contractions.
Comment 3 Frédéric Buclin 2013-05-30 03:53:10 PDT
(In reply to Byron Jones ‹:glob› from comment #0)
> this behaviour was changed by bugzilla 4.2 -- older versions don't treat '
> as a quote character.

4.2 correctly parses strings. 4.0 and older were totally broken.
Comment 4 Frédéric Buclin 2013-07-22 11:42:07 PDT
Comment on attachment 755773 [details] [diff] [review]
patch v1

This patch is useless. parse_line() does its job correctly. If I type:

"no, I can't"

(with quotes) in the QS field, then ' isn't seen as a quote character. Same if I type:

summary:"no, I can't"

Same if I type:

summary:'double quote " is legal'


Typing a whole string or a word containing quotes without quotes is a syntax error, IMO. Also, I use single quotes as separator if I want to look for double quotes inside strings and vice versa, and so your patch would break that.

This bug is WONTFIX to me, else you will break the current behavior for those who use single quotes as separator, e.g. in the case I mentioned above.
Comment 5 Byron Jones ‹:glob› 2013-07-22 21:11:20 PDT
i think the instances of people using an apostrophe in a word contraction would far exceed that of people using them as a quote character.  i don't think it's unreasonable to expect to be able to type common english words into a search field without needing to escape them.
Comment 6 Dave Miller [:justdave] (justdave@bugzilla.org) 2013-08-28 10:04:48 PDT
I think it's fine to treat them as quote characters, if they have whitespace or begining/end of the string on at least one side of them.  Embedded in a word is obviously being used as an apostrophe.
Comment 7 Byron Jones ‹:glob› 2013-08-28 10:20:24 PDT
Created attachment 796772 [details] [diff] [review]
877545_2.patch

this revision treats apostrophes as quotes only on string or word boundaries.
Comment 8 Frédéric Buclin 2013-08-28 11:23:39 PDT
(In reply to Byron Jones ‹:glob› from comment #2)
> >     # to sanely search for contradictions. As this behavour isn't
> 
> of course i mean contractions.

You still didn't fix this typo.
Comment 9 Frédéric Buclin 2013-08-28 11:34:19 PDT
Comment on attachment 796772 [details] [diff] [review]
877545_2.patch

QS is totally broken. It's now unable to parse foo:can't correctly. foo: is no longer seen as a field name.
Comment 10 Byron Jones ‹:glob› 2013-08-28 22:33:23 PDT
Created attachment 797072 [details] [diff] [review]
877545_3.patch
Comment 11 Byron Jones ‹:glob› 2013-08-28 22:34:52 PDT
amusing side-note: the sample query in the "unbalanced quotes" error has unbalanced quotes due to this bug:

If you use quotes to enclose strings, make sure both quotes are present. If you really want to look for a quote in a string, type \" instead of ". For instance, "I'm so \"special\", really" (with quotes) will be interpreted as I'm so "special", really.
Comment 12 mail 2013-09-13 05:01:28 PDT
Comment on attachment 797072 [details] [diff] [review]
877545_3.patch

I don't think it is working correct. If I quick search "ALL summary:bug" (without quotes) it returns what I expect.

If I quick search "ALL summary:it's" (again without quotes), it searches:

    Product: summary:it's
    Component: summary:it's
    Alias: summary:it's
    Summary: summary:it's
    Whiteboard: summary:it's

which seems incorrect.
Comment 13 Byron Jones ‹:glob› 2013-09-17 06:57:11 PDT
(In reply to Simon Green from comment #12)
> I don't think it is working correct. If I quick search "ALL summary:bug"
> (without quotes) it returns what I expect.
> 
> If I quick search "ALL summary:it's" (again without quotes), it searches:
> 
>     Product: summary:it's
>     Component: summary:it's
>     Alias: summary:it's
>     Summary: summary:it's
>     Whiteboard: summary:it's
> 
> which seems incorrect.

odd, when i perform that search: buglist.cgi?quicksearch=ALL+summary%3Ait's
it searches: Summary: it's

are you able to provide the query_string which buglist.cgi is getting?
Comment 14 mail 2013-09-17 16:09:08 PDT
(In reply to Byron Jones ‹:glob› from comment #13)
> odd, when i perform that search: buglist.cgi?quicksearch=ALL+summary%3Ait's
> it searches: Summary: it's

Odd indeed. When I tried it again this morning, it worked as expected.
 
Have changed my review decision.
Comment 15 Byron Jones ‹:glob› 2013-09-18 01:00:23 PDT
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Search/Quicksearch.pm
Committed revision 8739.

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/4.4/
modified Bugzilla/Search/Quicksearch.pm
Committed revision 8607.

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