367070 - new browser window is broken if it is opened from a window in which the findbar is open ('Error: this._browser has no properties').

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect, P2)

Description

[(not reading, please use [e-mail] instead)]

personal bookmarks toolbar blank if I open a new browser window from a browser when when the find bar is showing.

in my js console:

Error: this._browser has no properties

Source File: chrome://global/content/bindings/findbar.xml

Line: 641

I'm using my own (slightly old, debug trunk):

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070105 Minefield/3.0a2pre

i'll update, and rebuild and see if I still see this problem.

Comment 1

[Phil Ringnalda (:philor)]

You'll still see it, it was busted in my 2006-11-26 17:00 build so the findbar widget's always done that, and apparently no trunk tester has opened a new window over a month and a half.

Comment 2

[(not reading, please use [e-mail] instead)]

thanks phil.  I double checked, and I see this regression with the most recent trunk binaries, but not with fx 2.0.0.1.

Comment 3

[Asaf Romano (gone)]

Attachment: patch

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 251587 [details] [diff] [review]
patch

>Index: toolkit/content/widgets/findbar.xml

You missed a couple of findbar._browser references in the findbar-textbox binding (atm they only work because open() calls _updateCaseSensitivity() which calls the "browser" getter). r=me with those fixed.

Comment 5

[Asaf Romano (gone)]

Attachment: wrong patch

mozilla/toolkit/content/widgets/findbar.xml 1.6

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 251641 [details] [diff] [review]
wrong patch

This isn't what you checked in, thankfully :)

Comment 7

[Asaf Romano (gone)]

Attachment: as checked in

My manage-multiple-trees skills need some love indeed

Comment 8

[Robert Sayre]

this should not have been r+'d without a test. If a test case can be written for bug 288254, one can be written for this.

Comment 9

[Asaf Romano (gone)]

I will make sure to write this (these) tests once we have the necessary framework in place.

Comment 10

[Robert Sayre]

(In reply to comment #9)
> I will make sure to write this (these) tests once we have the necessary
> framework in place.

That is not the policy. Refer to

<http://www.mozilla.org/projects/toolkit/review.html>

"If a unit test framework is needed but is not yet available, the developer should write appropriate test code and commit it. A bug should be filed on the needed test framework."

Comment 11

[Asaf Romano (gone)]

Attachment: unit test

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

This bug has regressed Find-as-you-type: there is no response to / or '. Seen on Windows and Linux (no mac to test with) and there aren't any messages in the Error Console.

I tested with the first hourly build after this landed, which only had this checkin. 

Comment 13

[Asaf Romano (gone)]

Attachment: regression fix

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 251819 [details] [diff] [review]
regression fix

>Index: toolkit/content/widgets/findbar.xml

>+        setTimeout(function(aSelf) { aSelf.browser = aSelf.browser), 0, this);

Wrong end bracket there, and missing semicolon. Please attach this patch to a new bug that blocks this one.