429498 - Location bar does not search consistently (matches 1-after a CamelCase)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Andrés Delfino]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041606 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041606 Minefield/3.0pre

Location bar does not search consistently. It's kind of weird, so I'll just write the steps to reproduce this.

I'm requesting blocking since this is the main feature to ship with Firefox 3, so it should be as clean as possible.

Reproducible: Always

Steps to Reproduce:
1. Bookmark Bugzilla.
2. Type ugz.
3. Type zil.
Actual Results:  
Step 2 shows Bugzilla as an auto-completion suggest. Step 3 does not.

Expected Results:  
Both, step 2 and 3, should show Bugzilla as a suggest.

Comment 1

[Ed Lee :Mardak]

If you want to "zil" to match "bugZILla", turn off this pref:

browser.urlbar.matchOnWordBoundary

Comment 2

[Ed Lee :Mardak]

Oh, about it matching on "ugz", it's because of the conservative nature of word boundary matching to be able to match on CamelCase.

Comment 3

[Andrés Delfino]

(In reply to comment #2)
> Oh, about it matching on "ugz", it's because of the conservative nature of word
> boundary matching to be able to match on CamelCase.
> 

Could you please explain again?

Since Bugzilla has no mixed case, I see no reason for differentiate between ugz, and zil.

Comment 4

[Ed Lee :Mardak]

I had a change in heart ;)

Comment 5

[Ed Lee :Mardak]

Attachment: v1

..and thought of a 1-line solution that passes existing testcases and the new one of course. :)

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

I've actually run into this in other use cases, but wasn't smart enough to determine that it was mixed case causing it. Even I'd feel confident r+'ing this patch, it's that simple :)

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Turns out I totally misinterpreted what this patch does based on my own desire to have matching on word boundaries act as a priority, not a pruning.

Regardless, the inconsistency should be fixed.

Comment 8

[Ed Lee :Mardak]

A build to play with:
https://build.mozilla.org/tryserver-builds/2008-04-17_09:[e-mail]-nocamel.1after/

Comment 9

[Ed Lee :Mardak]

(In reply to comment #0)
> Expected Results:  
> Both, step 2 and 3, should show Bugzilla as a suggest.
Just to be clear, the patch doesn't do this.

The patch makes it so *both* "ugz" and "ila" *don't* show up as results because right now "word boundary matching" is treating "B" as a word boundary when it could do better and know that B is a special word boundary that shouldn't let the next character match on /1-after/ it.

Comment 10

[Simon Bünzli]

FYI: An edge case which will get slightly edgier is www.rottentomatoes.com (with an all uppercase title) where IIUC "otten" won't match but "tten" will.

Edward: What's wrong with considering the character's context in the word boundary check itself? E.g. the current character is at a word boundary when at least one of the following conditions is true

(1) the current character is the first in the string
(2) the current character is non-alphabetic
(3) the previous character is non-alphabetic
(4) the current character is upper case and the previous one isn't

Comment 11

[Ed Lee :Mardak]

Attachment: v1.1

Same as v1 but updated some comments.

Comment 12

[Dietrich Ayala (:dietrich)]

Comment on attachment 316352 [details] [diff] [review]
v1.1

looks fine, r=me.

Comment 13

[Ed Lee :Mardak]

Comment on attachment 316352 [details] [diff] [review]
v1.1

a1.9? for 1 line fix to make word boundary matching more consistent for CamelCase stuff with testcase

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 316352 [details] [diff] [review]
v1.1

a=mconnor on behalf of 1.9 drivers

Comment 15

[Ed Lee :Mardak]

Checking in toolkit/components/places/src/nsNavHistoryAutoComplete.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp,v  <--  nsNavHistoryAutoComplete.cpp
new revision: 1.57; previous revision: 1.56
done
Checking in toolkit/components/places/tests/autocomplete/test_word_boundary_search.js;
/cvsroot/mozilla/toolkit/components/places/tests/autocomplete/test_word_boundary_search.js,v  <--  test_word_boundary_search.js
new revision: 1.4; previous revision: 1.3
done

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using the testcase in comment 0 with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042404 Minefield/3.0pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008042407 Minefield/3.0pre

-and-

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008042404 Minefield/3.0pre