998773 - Find bar: "Match Case" should prevent curly quotes from matching straight quotes

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect, P5)

Description

[André]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140314220517

Steps to reproduce:

When I'm searching for a typewriter double quote (") also where found quotation mark (“)


Actual results:

As search results I get (also) quotation mark (“). That is the quotation mark, that in german marks the end of quotation.


Expected results:

Normally quotation marks (“) should not be displayed as search result when I'm searching for typewriter double quote (").

This issue exists not so long ~ maybe since a half year...

Comment 1

[:Gijs (he/him)]

Tom, any idea about when this changed and if it was a conscious change?

Comment 2

[Tom S. (please needinfo tschuster)]

This seems like vaguely useful behavior, because those quotation marks are hard to find. I can't really pinpoint the code for this right now. I would imagine it's somewhere in nsTypeAheadFind.

Comment 3

[André]

(In reply to Tom Schuster [:evilpie] from comment #2)
> This seems like vaguely useful behavior [...]

No, it's very unpractical/inconvenient when you search for typewriter double quote (") - because they are typographically incorrect (especially in german language) - and you will find quotation marks (“)!

Comment 4

[Alice0775 White]

Regression window(m-c):
Good:
http://hg.mozilla.org/mozilla-central/rev/d7d304e28da9
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20130917172619
Bad:
http://hg.mozilla.org/mozilla-central/rev/44f4567699ff
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20130917194519
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d7d304e28da9&tochange=44f4567699ff

Regressed by:
7a710c502b49	Matt Brubeck — Bug 276757 - Find in page should match curly quotes to straight quotes [r=jst]

Comment 5

[Matt Brubeck (:mbrubeck)]

As noted in bug 276757, this is intentional behavior.

This is better in the case where the user is searching for some text like « the project’s “openness" » and not know necessarily know whether it is typeset with curly or straight apostrophes/quotes, or how to type the different varieties.

I understand why it's not better in the case where you are specifically trying to identify mis-typeset characters, as in comment 3.  I think this is a less common case, so it should not determine the default behavior, but we could support it as an option.  The easiest step would be to add a hidden pref (which could be toggled in about:config or by an add-on).  Maybe it would even make sense for the "Match Case" toggle to turn on "strict" matching of punctuation as well as letters.

Note: other browsers (e.g. Chrome) also treat curly and straight quotation marks as matches when searching in a page.

Comment 6

[Philip Chee]

(Impractical) Use some sort of google-lite syntax e.g.
+"

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to Matt Brubeck (:mbrubeck) from comment #5)

> The easiest step would be to add a hidden
> pref (which could be toggled in about:config or by an add-on).

Though only a minority of potential users will ever discover this.

>  Maybe it
> would even make sense for the "Match Case" toggle to turn on "strict"
> matching of punctuation as well as letters.

IMO, this would be the more useful approach. I don't think we can expect to explain the precise differences between stricter and looser matching in the default UI, but "Match Case" is a widely-understood concept, and extending its semantics to other strict vs loose matching seems reasonable to me.

Comment 8

[Michael Bednarek]

(In reply to Matt Brubeck (:mbrubeck) from comment #5)
> As noted in bug 276757, this is intentional behavior.
[snip] 
> Note: other browsers (e.g. Chrome) also treat curly and straight quotation
> marks as matches when searching in a page.

True, and that's why I couldn't use Chrome for certain tasks. Until recently, I could use Firefox which didn't conflate typographical and straight quote signs and apostrophes. The new behaviour needs some kind of user configuration control. In the short term and to get it out quickly, about:config will do, whereas "Match case" conflates again two unrelated issues. Why not have a new tick box in Menu:Tools->Options->Content: "Search curly and straight apostrophes & quote signs combined" ?

Comment 9

[Jonathan Kew [:jfkthame]]

(In reply to Michael Bednarek from comment #8)

> Why not have a new tick box in Menu:Tools->Options->Content: "Search
> curly and straight apostrophes & quote signs combined" ?

Because this is only the tip of an almost indefinitely-large iceberg, and we can't deal with it one chip at a time. Bug 276757 just dealt with a few quote-mark characters, but there are many, many other examples where an option for "looser" matching would be beneficial to users most of the time.

How about matching various kinds of hyphens and dashes: -, –, —, ‑, ‒?

How about CJK ｆｕｌｌｗｉｄｔｈ and halfwidth variants?

How about the precomposed and decomposed versions of accented characters? How about ignoring diacritics, so that searching for "naive" can match "naïve"?

How about ignoring invisible control characters such as soft-hyphen, zero-width joiner, etc?

All these - and more - would be legitimate forms of loose matching, and in most cases the "looser" behavior would be helpful to users, who are usually looking for text with a particular meaning, rather than a precise encoded form.

At the Firefox user interface level, I don't think it's feasible to provide options to control each of these behaviors individually, but a single option that chooses "strict" or "loose" matching would be OK.

Maybe we could have two options, one for case-sensitivity and another for other forms of loose matching, but I fear this might be confusing to many users. Extending the meaning of "Match Case" (and possibly renaming it as something like "Exact Match", with a tooltip that clarifies a little more, "Requires capitalization, diacritics, quotes and dashes, etc. to match exactly"?) might be the best compromise between simplicity and control. But this is really a decision that needs to be considered by the Firefox UX team.

Comment 10

[Mike de Boer [:mikedeboer]]

(In reply to Jonathan Kew (:jfkthame) from comment #9)
> How about matching various kinds of hyphens and dashes: -, –, —, ‑, ‒?
> 
> How about CJK ｆｕｌｌｗｉｄｔｈ and halfwidth variants?
> 
> How about the precomposed and decomposed versions of accented characters?
> How about ignoring diacritics, so that searching for "naive" can match
> "naïve"?
> 
> How about ignoring invisible control characters such as soft-hyphen,
> zero-width joiner, etc?

*drool* I sooo want this. Is there already a bug on file to implement this fuzzy matching?

BTW, this already looks like a pretty much exhaustive list, i.e. spec.

Comment 11

[Jonathan Kew [:jfkthame]]

(In reply to Mike de Boer [:mikedeboer] from comment #10)
> *drool* I sooo want this. Is there already a bug on file to implement this
> fuzzy matching?

See, for example, bugs 202251, 477372, 640856, 648521; there may well be others.

Comment 12

[Justin]

i would like to work on it

Comment 13

[Matt Brubeck (:mbrubeck)]

Great! You can find the relevant code in bug 276757.  In particular, the APOSTROPHE and QUOTE transformations here should happen only when `mCaseSensitive` is false:
http://hg.mozilla.org/mozilla-central/file/29fbfc1b31aa/embedding/components/find/nsFind.cpp#l1113

For questions or help getting started, feel free to comment in this bug or contact me by email or on irc.mozilla.org.  You can also find general developer documentation here:
https://developer.mozilla.org/en-US/docs/Introduction

Comment 15

[Michael Bednarek]

Does the prolonged silence here indicate that the problem of not distinguishing between different kinds of apostrophes and quote signs (curly and straight) will not be fixed?

Comment 16

[Matt Brubeck (:mbrubeck)]

It looks like no one is working on this right now, but if anyone wants to fix it, see comment 13 for instructions.

Comment 17

[Steve Melia]

Attachment: 998773.patch

See attached for a patch.

This limits the behaviour introduced via Bug 276757 to when "match case" is unselected.

As in that bug; it seemed unwise to start dealing with the other normalisations proposed e.g. in comment 10; I think other cases are dealing with this in more depth e.g. Bug 202251

Comment 18

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8751909 [details] [diff] [review]
998773.patch

Looks good to me!  I'm not an official reviewer of this code; passing the review request to jst.

Comment 19

[Johnny Stenback (:jst)]

Comment on attachment 8751909 [details] [diff] [review]
998773.patch

Tying this to "Match Case" seems somewhat arbitrary to me TBH, but if this fixes a regression I'm cool with this landing. A better approach would seem to be to make sure we expose enough of this to extensions so that one could write an advanced find in page extension that could do all kinds of interesting stuff...

Comment 20

[Steve Melia]

Great thanks Johnny! I agree that "Match Case" is arbitrary and the primary motivation is to mitigate the previous change in behaviour. 

There are already extensions that allow for searching by regex etc. so I think that it would already be possible to implement fuzzy matching in an extension. Exposing innards to extensions does sound like a fun project though; do you think there is some more that could be done here?

Assuming we're OK to go ahead, may I have a try push if this is appropriate? and I can't seem to add the checkin-needed flag; perhaps because the case is not assigned to me?

Comment 21

[Mike de Boer [:mikedeboer]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bc0151cc207

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/80c71070fc08

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/80c71070fc08

Comment 24

[Johnny Stenback (:jst)]

Clearing needinfo since this already landed, and seems all good. Thanks!

Comment 25

[Ron Bentley [:rtb]]

I reproduced the original reported behavior in:
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160728203720

Search for double quote (") found the quotation mark (“) as well whether or not I checked "Match Case".

I can verify the resolution is working in 49.0b7:
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160825132718

In 49.0b7, if I select "Match Case" and search for double quote ("), it will NOT find the quotation mark (“).  If I uncheck "Match Case", it will.

[testday-20160826]