429723 - Empty findbar turns red and "Phrase not found" is displayed when "Highlight all" is enabled and search term is removed

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Pavlos Touboulidis]

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

Following the steps described below, the findbar incorrectly turns red and the "Phrase not found" is displayed while there's no phrase to search for (the findbar is empty).

Reproducible: Always

Steps to Reproduce:
1. Open any page that contains the letter 'a'
2. Hit Ctrl+F to open the Findbar, type 'a'
3. Click the highlight button so that it's on
4. Delete the letter 'a'
Actual Results:  
The findbar turned red

Expected Results:  
The findbar is empty so it should be white and the "Phrase not found" message not visible.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Regression range:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2005-12-16+13%3A00&maxdate=2005-12-16+23%3A00
Caused by Bug 313653, I think.

Comment 2

[Graeme McCutcheon [:graememcc]]

Attachment: Patch v1 [Checkin: Comment 4]

The findbar highlighting code shouldn't attempt to search for empty strings.

Comment 3

[Asaf Romano (gone)]

Comment on attachment 337864 [details] [diff] [review]
Patch v1
[Checkin: Comment 4]

r=mano

Comment 4

[Serge Gautherie (:sgautherie)]

Comment on attachment 337864 [details] [diff] [review]
Patch v1
[Checkin: Comment 4]

http://hg.mozilla.org/mozilla-central/rev/2321c54b2a8a

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b1pre) Gecko/20080927020752 Minefield/3.1b1pre ID:20080927020752

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080927033433 Minefield/3.1b1pre ID:20080927033433

Comment 6

[Graeme McCutcheon [:graememcc]]

Attachment: Test v1 (checked in)

Patch to test for this.

(Note I've used the opportunity to do some trivial code cleanup that is too trivial to justify it's own bug: it reorders the tests in the makefile into bug# order, corrects my incorrect licence headers and removes a spurious comment from bug 451286's test).

Comment 7

[Marcia Knous [:marcia]]

nominating for intestsuite based on Comment 6

Comment 8

[Asaf Romano (gone)]

Comment on attachment 343247 [details] [diff] [review]
Test v1 (checked in)

r=mano

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 343247 [details] [diff] [review]
Test v1 (checked in)

http://hg.mozilla.org/mozilla-central/rev/23a15042356f

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

With the automated test we should be fine.