Closed
Bug 78419
Opened 24 years ago
Closed 24 years ago
Find is broken in view source, message pane, and message window
Categories
(SeaMonkey :: General, defect)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9.1
People
(Reporter: jag+mozbugs, Assigned: sfraser_bugs)
References
Details
Attachments
(5 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
viewsource.js is still calling appCore.find and appCore.findNext.
Comment 1•24 years ago
|
||
Comment 2•24 years ago
|
||
Life got too annoying without this working.... So I basically copied the
navigator.js code.
It may make sense to refactor some code that navigator and viewsource share into
a separate JS file...
Comment 3•24 years ago
|
||
That's how i have it in my tree too. However, moving the find code to another
file I think makes sense.
Assignee | ||
Comment 4•24 years ago
|
||
Question: does the message pane in mailnews, or the standalone message window
have Find?
Comment 5•24 years ago
|
||
Yes, they both do. It's in the Search menu in both windows.
Assignee | ||
Comment 6•24 years ago
|
||
OK, so I need to fix them too.
Summary: view-source wasn't updated for new find code → Find is broken in view source, message pane, and message window
Comment 7•24 years ago
|
||
something else, if one chooses "find again" without searching before, i get a
nice fat assertion.
in BrowserFindAgain(), you can add first:
+if (!BrowserCanFindAgain())
+ BrowserFind();
+else
if (window.findDialog)
That fixes this.
nc4's find again isn't enabled by default. [i'm assuming if i used find it
would be...]
Assignee | ||
Comment 9•24 years ago
|
||
Would it be good to move the Find methods into utilityOverlay.js?
Assignee | ||
Comment 10•24 years ago
|
||
Assignee | ||
Comment 11•24 years ago
|
||
Assignee | ||
Comment 12•24 years ago
|
||
These patches move find utility functions into utilityOverlay.js, and move
mailnews to using the new find for message panes.
alecf: can I get an sr?
jag/doron: can I get an r?
Target Milestone: --- → mozilla0.9.1
Comment 13•24 years ago
|
||
would you mind adding a ; here?
+var gBrowser = null
var appCore = null;
(yes it's not strictly required as end of lines are also instr seps)
this code is repeated, but i'm guessing a function would be more expensive
+ var focusedWindow = document.commandDispatcher.focusedWindow;
+ if (!focusedWindow || focusedWindow == window)
+ focusedWindow = window._content;
Comment 14•24 years ago
|
||
(I'd kinda prefer you use a function as well because focus is so messy as it is
- if we ever have to fix the focusWindow junk, it would be nice just to fix it
in one place...
sr=alecf - with timelesses's issues...thanks for fixing this simon
adding seth to CC just so he knows this is going on
Comment 15•24 years ago
|
||
r=doron with timeless's changes, much better idea than creating it's own js file.
btw, does this also fix the case where one gets an assertion when you choose
find again without any searchstring specified?
Reporter | ||
Comment 16•24 years ago
|
||
Is utilityOverlay.js really the place to put it?
http://lxr.mozilla.org/seamonkey/search?string=utilityOverlay.xul%22
That shows it's overlayed into a lot of places.
Wouldn't it be cleaner to create findUtils.js and add those where needed?
Assignee | ||
Comment 17•24 years ago
|
||
I hate to add more .js files; the overhead of sucking in a new JS file seems
larger than that of bloating utilityOverlay.js a bit. I agree that this probably
should not go into an overlay that is used by dialogs, however. Is there another
alternative?
Comment 18•24 years ago
|
||
I believe the xul cache will keep a compiled version of a .js file in memory,
making adding an additional .js file to be small.. there are so many consumers
of utilityOverlay, it really seems better to burden the few (2? 3?) consumers of
the "find" feature with an extra .js file, than it is to burden most windows
with extra JS
Assignee | ||
Comment 19•24 years ago
|
||
OK, i'll break it out. doron: yes, this fixes the Find Next assertion.
Status: NEW → ASSIGNED
Assignee | ||
Comment 20•24 years ago
|
||
OK, where should findUtils.js live? The Find dialog stuff is currently in
toolkit.jar/content/global/, but the utility overlay stuff lives in comm.jar/
content/communicator.
Comment 21•24 years ago
|
||
I would say in the "communicator" component, as opposed to global (global tends
to deal with more finer granularity components)
Assignee | ||
Comment 22•24 years ago
|
||
Assignee | ||
Comment 23•24 years ago
|
||
Assignee | ||
Comment 24•24 years ago
|
||
Final answer patches just attached.
Reporter | ||
Comment 25•24 years ago
|
||
Is this really your final answer?
Comment 26•24 years ago
|
||
I just saw that in the old browserinstance way, "find again" when no
searchstring is available would launch the find dialog. This is not present anymore.
Comment 27•24 years ago
|
||
actually, we might best just grey the menuitem out until there is a search string.
Assignee | ||
Comment 28•24 years ago
|
||
This patch does attempt to grey the menu item, but since no-one updates the state
of cmd_find and cmd_findAgain, the routine is never called.
This is my final answer.
Comment 29•24 years ago
|
||
sr=sspitzer on the mailnews part.
Reporter | ||
Comment 30•24 years ago
|
||
r=jag if you fix the indentation in |canFindAgainInPage()|.
Assignee | ||
Comment 31•24 years ago
|
||
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 32•24 years ago
|
||
*** Bug 33190 has been marked as a duplicate of this bug. ***
Comment 33•24 years ago
|
||
If I do Ctrl-U to view source of a mail message, I have to click in the source
window before Ctrl-F will work.
Feels like a focus problem so should probably be a new bug, though (if there
isn't one already).
Comment 34•24 years ago
|
||
Bug 80598 filed on the focus issue (the one preventing find from working
immeditely in view source).
This one is verified fixed for me with 2001-05-11-08 on Linux, by the way.
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•