Closed Bug 46869 Opened 20 years ago Closed 19 years ago
Embedding apps need Find functionality
This is not the level we want to expose. -Conrad to convert http://lxr.mozilla.org/seamonkey/source/embedding/browser/powerplant/source/CFin dComponent.h into an xpidl interface and move both the interface and the implementation into the core. -FindAll to be removed from the interface. -nsString& and PRBool& to be converted to XPCOM equivalents.
By "core", do we mean in terms of embedding or SeaMonkey? If it's embedding - since we have embedding/base, I think a good place would be embedding/extras/ find. There is reason to put it in the core of SeaMonkey though. The find algorithm mine uses was taken from xpfe/components/find/ It adds entire word matching though. SeaMonkey's could be layered on top of mine. It would handle the XUL UI and call on my component to do the search.
Status: NEW → ASSIGNED
changing to new email
re-accepted at new email
Status: NEW → ASSIGNED
per email with Jud, changing nsbeta3+ to nsbeta3- on all "embed" keyword bugs since embedding changes will not be made in the mn6 branch. If you feel this bug fix needs to go into mn6 branch, please list the reasons/user impact/risk and nominate for rtm. Thanks.
Whiteboard: [nsbeta3+] → [nsbeta3-]
Updating QA Contact
QA Contact: jrgm → mdunn
Summary: nsITextServices should not be what we use for find dialog support. → Embedding apps need Find functionality
Kin, can you review? The nsWebBrowserFind code was mainly copied straight from your code. I just put the needed functionality in the webbrowser lib since that's what embedding clients use.
Conrad, looks good. r=kin
Vidur, can you sr? Main question is my impl of nsWebBrowser::GetFocusedWindow(). This needs to be used so that finding in framesets works on the "focused" frame. It does work but I'm not sure if I'm getting the focused DOM window in the best way.
Chris (Saari) - Can you take a look at the nsWebBrowser::GetFocusedWindow() since you're dealing w/ focus issues?
So can we change find in mozilla to use the same code? I hate to see code duplicate for mozilla/embedding that could be shared.
The bulk of the code is in common - in nsIFindAndReplace which this code uses. The rest could be made common if we had Find(), etc. on nsIDOMWindow.
Your implementation looks fine, but nsIWebBrowserFocus (which has IDL checked in, but I havn't tested the implementation yet) has attribute nsIDOMWindow focusedWindow; as part of it. Shouldn't this just live there? If you want to check this in I can retrofit it when I land my implementation.
That is where my GetFocusedWindow() lives. The method was aleady there but empty on nsWebBrowser::nsIWebBrowserFocus. I just filled it in.
Doh, sorry, I should pay more attention. Proceed.
looks good. I'm assuming we should switch over to this interface in mozilla-the-browser as well? Conrad can you file a bug against that? my one comment on the interface, is why have void findNext(out boolean didFind); when you could have boolean findNext(); ? makes it a whole lot easier from a scripter's point of view...
> I'm assuming we should switch over to this interface in mozilla-the-browser as > well? I'm not sure how much the two have in common other than what they do share - use of the nsIFindAndReplace component. The Find component used in the browser exists mainly to manage interraction between the JS find window and the find implementation whereas the point of this one is to have no tie to any UI. > my one comment on the interface, is why have > void findNext(out boolean didFind); > when you could have > boolean findNext(); ? Good point - I'll make the change.
Some nits: 1) In nsWebBrowser::GetFocusedWindow, you do an extra QI - you should be able to QI directly from nsIDOMWindow to nsPIDOMWindow (we should get rid of the latter interface anyway - it should get merged with nsIDOMWindowInternal). 2) In nsWebBrowserFindImpl::MakeTSDocument, can't you get the document directly from the nsIDOMWindow using nsIDOMWindow::GetDocument()? 3) I'm not a big fan of accessors returning references to members (the methods that allow access to the falgs in nsWebBrowserFindImpl()). Just a personal preference, I guess. The first couple of issues should be fixed. Other than that, looks good. sr=vidur.
> I'm not sure how much the two have in common other than what they do share - use > of the nsIFindAndReplace component. The Find component used in the browser > exists mainly to manage interraction between the JS find window and the find > implementation whereas the point of this one is to have no tie to any UI. IMO the implemtation of the Find component in Mozilla is really broken. There is C++ code that brings up a JS dialog, which seems just back-asswards to me. This should be rewritten so that the dialog is invoked from XUL/JS, and just talks to the XPIDLsed find interfaces.
Incorporated Alec's suggestion about boolean findNext(); as well Vidur's points.
as a side note, simon is right :) you mind filing a bug on the brokeness? other than that, sr=alecf with that.
Yes, I agree with Simon too. I'll file a bug.
Fixed checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Correction: Changing QA contact for the Embed API bugs to David Epstein.
QA Contact: mdunn → depstein
find is in embedding apps (mfcembed, testembed).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.