Closed Bug 877545 Opened 7 years ago Closed 6 years ago

quicksearch shouldn't treat apostrophes as quote characters

Categories

(Bugzilla :: Query/Bug List, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 4.4

People

(Reporter: glob, Assigned: glob)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch patch v1 (obsolete) — Splinter Review
Assignee: query-and-buglist → glob
Attachment #755773 - Flags: review?(LpSolit)
>     # to sanely search for contradictions. As this behavour isn't

of course i mean contractions.
(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.
OS: Mac OS X → All
Hardware: x86 → All
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.
Attachment #755773 - Flags: review?(LpSolit) → review-
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.
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.
Attached patch 877545_2.patch (obsolete) — Splinter Review
this revision treats apostrophes as quotes only on string or word boundaries.
Attachment #755773 - Attachment is obsolete: true
Attachment #796772 - Flags: review?(simon)
(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 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.
Attachment #796772 - Flags: review?(simon) → review-
Attached patch 877545_3.patchSplinter Review
Attachment #796772 - Attachment is obsolete: true
Attachment #797072 - Flags: review?(simon)
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 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.
Attachment #797072 - Flags: review?(simon) → review-
(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?
Attachment #797072 - Flags: review- → review+
(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.
Status: NEW → ASSIGNED
Flags: approval?
Flags: approval4.4?
Target Milestone: --- → Bugzilla 4.4
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval+
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.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Blocks: 918647
You need to log in before you can comment on or make changes to this bug.