Closed Bug 953537 Opened 8 years ago Closed 8 years ago

Add a search bar in conversation window

Categories

(Instantbird :: Conversation, enhancement)

0.1.2
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: romain, Assigned: romain)

References

()

Details

Attachments

(3 files, 1 obsolete file)

*** Original post on bio 86 at 2008-08-09 23:50:00 UTC ***

Make Ctrl-F in conversation window to open a search bar like the one in Mozilla Firefox.

"Should not be hard because all is shared in the toolkit" @fqueze
*** Original post on bio 86 at 2008-08-10 00:53:49 UTC ***

Quick note: It would be easier to look at how this is implemented in the "View Source" window of Firefox than in the main browser window; the code is much shorter there.

http://mxr.mozilla.org/seamonkey/source/toolkit/components/viewsource/content/viewSource.xul
Attached patch patch (obsolete) — Splinter Review
*** Original post on bio 86 as attmnt 63 at 2008-08-16 02:06:00 UTC ***

Here is a proposed patch.
I think that bar is too big, and on a standard conversation window of instantbird, the "Reached end of page, continued from top" label is cut.

Maybe a copy of findbar binding and change some things / elements (?)

I am not sure this is what we expect.
Assignee: florian → romain
Status: NEW → ASSIGNED
*** Original post on bio 86 at 2008-08-16 13:10:30 UTC ***

When I hit Ctrl-f, the search field is not focused and I see:
JavaScript strict warning: chrome://global/content/bindings/findbar.xml, line 721: reference to undefined property this.nsITypeAheadFind
JavaScript error: chrome://global/content/bindings/findbar.xml, line 721: this.nsITypeAheadFind is undefined

When I hit Ctrl-f a second time, the search field get the focus, and I see:
JavaScript strict warning: chrome://global/content/bindings/findbar.xml, line 171: reference to undefined property findbar.nsISelectionController
JavaScript error: chrome://global/content/bindings/findbar.xml, line 171: findbar.nsISelectionController is undefined

Then each time I type in the search field, I see:
JavaScript strict warning: chrome://global/content/bindings/findbar.xml, line 1101: reference to undefined property this.nsITypeAheadFind
JavaScript error: chrome://global/content/bindings/findbar.xml, line 1101: this.nsITypeAheadFind is undefined

When closing the findbar manually (and probably automatically too, I saw this warning without doing anything), I got:
JavaScript strict warning: chrome://global/content/bindings/findbar.xml, line 771: reference to undefined property this.nsISelectionController
JavaScript error: chrome://global/content/bindings/findbar.xml, line 771: this.nsISelectionController is undefined

Otherwise, it seems to work as expected.

> the "Reached end of page, continued from top" label is cut.

I don't see it at all (even with if I maximize the conversation window on a 24" monitor).

> I think that bar is too big

The size seems good to me. But when later we will have a toolbar with formatting icons, we will probably want to hide it when the findbar is visible.


So, please try to fix the above JS errors.
Make sure the search field is focused when the findbar is opened, and that the findbar closes automatically. (All of this seems related to the above mentioned errors).
Attached patch patch v2Splinter Review
*** Original post on bio 86 as attmnt 64 at 2008-08-16 19:26:00 UTC ***

There is still an error when creating a new tab.
On my opinion there is something wrong in findbar.xml (toolkit), because we can't create a new tab without getting an error / a warning.

If I define findbar.browser before findbar construct, I get an error with findbar.xml line 316
> this._findField.value = this._browser.fastFind.searchString;

because this._findField is defined in constructor.


If I define findbar.browser after findbar construct, I get warning on 
findbar.xml, line 299
> document.getElementById(this.getAttribute("browserid"));

findbar.browser is called at the end of the constructor by :
> setTimeout(function(aSelf) { aSelf.browser = aSelf.browser; }, 0, this);

and because our browser have no "id" attribute, there's a warning :
Warning: Empty string passed to getElementById()



Moreover I suggest to do searches beginning from bottom of conversation to top.
Comment on attachment 8351807 [details] [diff] [review]
patch

*** Original change on bio 86 attmnt 63 at 2008-08-16 19:26:21 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351807 - Attachment is obsolete: true
*** Original post on bio 86 at 2008-09-24 21:48:50 UTC ***

Just pushed some changes based on this patch as 355:43b3e5cd830d.
Target Milestone: --- → 0.1.3
*** Original post on bio 86 as attmnt 102 at 2008-09-25 19:04:00 UTC ***

This is the patch that I just pushed (as 358:253fea34bbfc) in our set of patches to apply against the code of Mozilla-central.
Attached patch Catch the error.Splinter Review
*** Original post on bio 86 as attmnt 104 at 2008-09-26 22:41:00 UTC ***

Add a try/catch to silence out the error mentioned in comment 4.
*** Original post on bio 86 at 2008-09-26 22:47:31 UTC ***

With that last patch (pushed as 362:ee1822a65f45), I think this bug is now FIXED.

For future reference, the errors that we had to workaround seem to be caused by https://bugzilla.mozilla.org/show_bug.cgi?id=297717
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.