Closed Bug 222305 Opened 21 years ago Closed 21 years ago

"Find in This Message" finds nothing until message text clicked on

Categories

(SeaMonkey :: MailNews: Message Display, defect)

x86
All
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: swick, Assigned: Bienvenu)

References

Details

Attachments

(3 files)

User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20030925 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5) Gecko/20030925 After clicking on a message header to see a message, I go to "Edit / Find in This Message" to search on text within that message. I always get the error "The text you entered was not found." even for a text string that is obviously in the message. I found that I actually had to click in the box containing the message text before "Find in This Message" would actually find anything. NOTE: The same is true if I bring up a message by double-clicking the header and bringing it up in a new window. Reproducible: Always Steps to Reproduce: 1. Click on any message in your Inbox 2. Go to dit / Find in This Message 3. Try to find text that obviously exists in the message without actually clicking in the box containing the message text. Actual Results: "The text you entered was not found." Expected Results: If I am looking at an email message, I expect that Mozilla should be able to search it without actually having to click in the email text. Thank you for taking the time to look at this. I searched and didn't find it reported, but my apologies if it has been.
Confirming on WinXP Moz 2003101604. OS -> All Severity -> Minor (easy workaround)
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
taking - this is a little bit more than a focus issue. If you pick wrap around, it will find the text as well. It's as if the starting point for the search isn't set correctly if the message pane does not have the focus.
Assignee: sspitzer → bienvenu
the browser has the same bug - if the url bar has the focus, and you do a "find in this page", it doesn't work, unless you have wraparound turned on.
Blocks: 229740
firebird does not have this bug, just the browser in the "big app". Brian, do you have any idea what might be going on here? Seth was wondering if it's an event state manager issue.
I'm not sure (yet) why it works for firebird. but the reason it doesn't work for mozilla mail or tbird is that nsDocShell::EnsureFind() calls SetCurrentSearchFrame() with the focused window if the focused window is our messagepane body, it works great. if the focused window is something else (thread pane, folder pane, quick search text bar), we end up passing the chrome window, and find fails. hmm, does using a top level chrome window for the current search frame *ever* make sense?
I've got a patch that appears to work, but it's nothing to be proud of. but maybe it will lead to something better. I don't think mCurrentSearchFrame should ever be a DOMChromeWindow. But if that's what has focus, EnsureFind() will set it to that. So I've put a check in nsWebBrowserFind::SetCurrentSearchFrame() Alternatively, the check could be in nsDocShell::EnsureFind()
Attached patch patch (deleted) — Splinter Review
FYI I see two other ways we can attack this: 1. Prevent EnsureFind form stomping over existing settings 2. Make findUtils cache the find instance (avoiding calling EnsureFind)
Actually nsIWebBrowserFind states that the current search frame must be a child of the root search frame - maybe we should enforce this.
> FYI I see two other ways we can attack this: > 1. Prevent EnsureFind form stomping over existing settings > 2. Make findUtils cache the find instance (avoiding calling EnsureFind) Any luck with either of those? For #1, we have to make sure we don't want it to stomp, as it sents the settings based on focus. (I'm sure we do that for a reason). for #2, EnsureFind is called by nsDocShell::GetInterface(), are you sure caching it is going to fix that? > Actually nsIWebBrowserFind states that the current search frame must > be a child of the root search frame - maybe we should enforce this. a child, or the same as, right? (the root can == the current) Any ideas on where we could enforce this? The thing that I keep wanting to know is why this works for firebird, but nothing else. (I don't think findUtils for mozilla/toolkit is any different, and I didn't see us sending focus to window.content or anything. but I didn't debug, so I was just eyeballing code in lxr)
Attached patch Focussed frame check (deleted) — Splinter Review
This checks that the focussed frame is within the docshell's tree. I don't know if it's the best way of checking though.
Having the FindInstData cache the webBrowserFind object seems like a fairly clean way to fix this. EnsureFind() will only be called when you access browser.webBrowserFind, as far as I can tell.
Attached patch Cache find instance (deleted) — Splinter Review
I am guessing here that the find instance generally lives at least as long as the xbl binding does...
will fixing browser.xml fix moz mail and tbird? we should probably make the same fix to firebird's version of browser.xml, too. do we need to fix the one in editor.xml? /toolkit/content/widgets/browser.xml, line 222 -- <property name="webBrowserFind" /toolkit/content/widgets/tabbrowser.xml, line 1204 -- <property name="webBrowserFind" /toolkit/content/widgets/editor.xml, line 72 -- <property name="webBrowserFind" /xpfe/global/resources/content/bindings/browser.xml, line 204 -- <property name="webBrowserFind" /xpfe/global/resources/content/bindings/editor.xml, line 72 -- <property name="webBrowserFind" /xpfe/global/resources/content/bindings/tabbrowser.xml, line 1365 -- <property name="webBrowserFind"
Editor/MsgCompose find is currently conditional on the focus, so it's a potential problem if you want to be able to find without focussing first.
FYI, I fixed this for thunderbird a couple months ago: Bug #227216 that just forces focus to the message pane when the find is invoked. Might be able to remove that code change if this bug gets fixed.
Scott, that change breaks find in the stand-alone message pane (I tried that too :-) ). It also doesn't seem to be on the trunk or on the m4 branch - could it have just been on some abandoned branch?
No I checked in a fix for the stand alone message pane as well the same day. I'll try to find the checkin to make sure it went into the trunk.
David, http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/mail/base/content&command=DIFF_FRAMESET&file=mailWindowOverlay.js&rev1=1.30&rev2=1.31&root=/cvsroot went into the trunk on december 1st. In thunderbird 12/23 build this works in both the stand alone message window and in the 3-pane for me. Is that not the case for you?
I don't see the fix on the trunk at all - in mozilla/mail/base/content/mailWindowOverlay.js, MsgFind() looks like this. function MsgFind() { findInPage(getFindInstData()); } it looks like you backed yourself out with the next rev, 1.32, to fix the stand-alone message case, but I don't know if there was a different fix. When I tried this on the tbird trunk, I did encounter this bug. I still have a bit of a high fever so I could be confused....
Attachment #138238 - Flags: review?(bryner)
Attachment #138238 - Flags: review?(bryner) → review+
Attachment #138238 - Flags: superreview+
I checked in to both versions of browser.xml, I hope that's OK :-)
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: