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 patchSplinter 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)
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.
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: