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)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla0.9.1

People

(Reporter: jag+mozbugs, Assigned: sfraser_bugs)

References

Details

Attachments

(5 files)

viewsource.js is still calling appCore.find and appCore.findNext.
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...
Keywords: patch, review
That's how i have it in my tree too. However, moving the find code to another file I think makes sense.
Question: does the message pane in mailnews, or the standalone message window have Find?
Yes, they both do. It's in the Search menu in both windows.
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
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...]
Would it be good to move the Find methods into utilityOverlay.js?
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
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;
(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
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?
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?
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?
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
OK, i'll break it out. doron: yes, this fixes the Find Next assertion.
Status: NEW → ASSIGNED
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.
I would say in the "communicator" component, as opposed to global (global tends to deal with more finer granularity components)
Attached patch mozilla/xpfe patch (deleted) — Splinter Review
Attached patch mozilla/mailnews/base/ patch (deleted) — Splinter Review
Final answer patches just attached.
Is this really your final answer?
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.
actually, we might best just grey the menuitem out until there is a search string.
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.
sr=sspitzer on the mailnews part.
r=jag if you fix the indentation in |canFindAgainInPage()|.
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
*** Bug 33190 has been marked as a duplicate of this bug. ***
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).
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.
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: