422490 - Throbber keeps spinning when typing in a new url (AwesomeBar keeps searching when there's no results)

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Ed Lee :Mardak]

Comment 1

[Ed Lee :Mardak]

Whoops.. totally forgot about the comments :p

If the user is typing out a new url or perhaps a data uri or something.. the autocomplete will keep trying to find pages through the WHOLE history even though it won't find anything.. especially when the user keeps adding more words.

We can optimize this by not doing the search if the previous search finished and found nothing.

Comment 2

[Ed Lee :Mardak]

This patch is on top of bug 395161 and bug 392143.. so while not true dependencies.. I'll need to unbitrot to not need them (the context) :(.

Comment 3

[Ed Lee :Mardak]

Attachment: v1

You probably won't be able to apply the patch without grabbing the other ones first.

Comment 4

[Ed Lee :Mardak]

Attachment: v1.1

Forgot to check for javascript: URIs. Yay unit tests ;)

Comment 5

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

Another human performance thing, which will affect perception of browser speed.

Comment 6

[Ed Lee :Mardak]

Attachment: v1.2

Unbitrot from moving the patch to the top of my stack as this is the most-blocking patch I have for dietrich right now.

Comment 7

[Dietrich Ayala (:dietrich)]

Comment on attachment 309178 [details] [diff] [review]
v1.2

>+  if (mAutoCompleteFinishedSearch &&
>+    prevMatchCount < (PRUint32)mAutoCompleteMaxResults &&
>+    !prevSearchString.IsEmpty() && Substring(mCurrentSearchString, 0,
>+      prevSearchString.Length()).Equals(prevSearchString) &&
>+    (StartsWithJS(prevSearchString) == StartsWithJS(mCurrentSearchString))) {

please line up conditions with the first, and put the substring all on one line for readability.

r=me, thanks!

Comment 8

[Ed Lee :Mardak]

Attachment: v1.3

(In reply to comment #7)
> >+  if (mAutoCompleteFinishedSearch &&
> >+    prevMatchCount < (PRUint32)mAutoCompleteMaxResults &&
> >+    !prevSearchString.IsEmpty() && Substring(mCurrentSearchString, 0,
> >+      prevSearchString.Length()).Equals(prevSearchString) &&
> >+    (StartsWithJS(prevSearchString) == StartsWithJS(mCurrentSearchString))) {
> please line up conditions with the first, and put the substring all on one line
> for readability.
Lined up and.. got rid of the Substring and replaced with StringBeginsWith(mCurrentSearchString, prevSearchString)

Comment 9

[Ed Lee :Mardak]

Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.272; previous revision: 1.271
done
Checking in toolkit/components/places/src/nsNavHistory.h;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.h,v  <--  nsNavHistory.h
new revision: 1.148; previous revision: 1.147
done
Checking in toolkit/components/places/src/nsNavHistoryAutoComplete.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryAutoComplete.cpp,v  <--  nsNavHistoryAutoComplete.cpp
new revision: 1.48; previous revision: 1.47
done

Comment 10

[Chris Blore]

VERIFIED Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008051202 Firefox/3.0

Comment 11

[Tracy Walker [:tracy]]

added test cases https://litmus.mozilla.org/show_test.cgi?id=9468 to litmus