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)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: swick, Assigned: Bienvenu)
References
Details
Attachments
(3 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
bryner
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
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.
Comment 1•21 years ago
|
||
Confirming on WinXP Moz 2003101604.
OS -> All
Severity -> Minor (easy workaround)
Severity: normal → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Assignee | ||
Comment 2•21 years ago
|
||
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
Assignee | ||
Comment 3•21 years ago
|
||
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.
Assignee | ||
Comment 4•21 years ago
|
||
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.
Comment 5•21 years ago
|
||
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?
Comment 6•21 years ago
|
||
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()
Comment 7•21 years ago
|
||
Comment 8•21 years ago
|
||
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)
Comment 9•21 years ago
|
||
Actually nsIWebBrowserFind states that the current search frame must be a child
of the root search frame - maybe we should enforce this.
Comment 10•21 years ago
|
||
> 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)
Comment 11•21 years ago
|
||
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.
Comment 12•21 years ago
|
||
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.
Comment 13•21 years ago
|
||
I am guessing here that the find instance generally lives at least as long as
the xbl binding does...
Comment 14•21 years ago
|
||
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"
Comment 15•21 years ago
|
||
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.
Comment 16•21 years ago
|
||
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.
Assignee | ||
Comment 17•21 years ago
|
||
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?
Comment 18•21 years ago
|
||
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.
Comment 19•21 years ago
|
||
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?
Assignee | ||
Comment 20•21 years ago
|
||
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....
Comment 21•21 years ago
|
||
Your right. I backed out that fix and fixed it a different way:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/mail/base/content&command=DIFF_FRAMESET&file=mail3PaneWindowCommands.js&rev1=1.9&rev2=1.10&root=/cvsroot
due to Bug #227335
Assignee | ||
Updated•21 years ago
|
Attachment #138238 -
Flags: review?(bryner)
Updated•21 years ago
|
Attachment #138238 -
Flags: review?(bryner) → review+
Assignee | ||
Updated•21 years ago
|
Attachment #138238 -
Flags: superreview+
Comment 22•21 years ago
|
||
I checked in to both versions of browser.xml, I hope that's OK :-)
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•