10/26 16 MN6 On any bugzilla page, use ctrl-F to search for any string that's only in a text input field. You will not find it.

Noticed something similar on linux for a while now. It seems one has to check the "Search Backwards" to find words. In order to find anything i first do a default search - search beeps to me - then i Search Backwards - and the word is found.

Find in page is supposedly handled by editor. Sending to beppe.

simon, would this be yours?

-> kin. prolly a dup.

This bug does not appear to be a duplicate. (At least I can't find a dup.) This is probably related to bug 51550 and bug 10470 in which the find finds stuff that it shouldn't. My theory was that it was finding stuff from the html file whether or not it was visible on the screen. That might explain why text areas are ignored, as the text is usually entered after the page is rendered. However, testing with some pre-filled-in text areas, find still ignores them. Also, if you hit ctrl-f in a text area it moves the cursor rather than bringing up a find dialog.

that is consistent with 4.x, but is a nice enhancement request, setting to future

This is an important feature for web applications that use large TEXTAREAs, like when viewing NOTES at Yahoo! Calendar. IE5 has this feature.

Kinda of a dup of bug 76374.

*** Bug 84650 has been marked as a duplicate of this bug. ***

*** Bug 104688 has been marked as a duplicate of this bug. ***

mass moving open bugs pertaining to find in page/frame to pmac@netscape.com as qa contact. to find all bugspam pertaining to this, set your search string to "AppleSpongeCakeWithCaramelFrosting".

remove self

*** Bug 135095 has been marked as a duplicate of this bug. ***

*** Bug 124989 has been marked as a duplicate of this bug. ***

See also bug 120568, "Find/Replace should be available for text fields in Navigator".

*** Bug 179858 has been marked as a duplicate of this bug. ***

Has any progress been made or work been done to address this bug? It's a definite problem for me, as an internal web application at our company uses large text fields extensively. I don't see much other than closing of duplicates done since 2001.

I'm afraid that I've had to go to IE for this type of application. And since I use wikis a fair bit, I've not used mozilla much for a while. I suppose I'm just not in the relevant market segment. Was there a second birthday party for the bug?

Selmer, is this really a P5 Future Enhancement? Sure, it was in October 2000, but it's 2003! Also - Should have keywords helpwanted, nsCatFood, nsbeta1 IMO. Last two comments concur...

Matthew, I don't get to determine the priority or milestone. It is definitely an enhancement request. The owner (kin) should put helpwanted if he wants help. I have added nsCatFood though. Besides voting for this bug, you might look for other duplicates and make sure they all get duped against this bug. At some point, a large number of dups will bring attention as well (there are currently 5 dups.)

topembed nominating

Since we don't have a request for this (yet) from an embeddor or from the composition point of view I'm going to topembed- this.

*** Bug 192645 has been marked as a duplicate of this bug. ***

*** Bug 194129 has been marked as a duplicate of this bug. ***

*** Bug 199894 has been marked as a duplicate of this bug. ***

*** Bug 200495 has been marked as a duplicate of this bug. ***

*** Bug 201179 has been marked as a duplicate of this bug. ***

*** Bug 201206 has been marked as a duplicate of this bug. ***

Teaking summary to help avoid dupes.

Some hints: First, textareas are intentionally skipped. I can't remember why that was, though I remember we discussed it and weren't sure what the right answer was. So the first step in fixing this is to stop skipping them intentionally. See the attached patch. However, applying this patch doesn't actually find matches in textareas. If you turn on DEBUG_FIND, you see that it finds the match, but then it fails this test: if (startParent && endParent && IsVisibleNode(startParent) && IsVisibleNode(endParent)) which according to the comments means "invisible or bad range". It turns out that IsVisibleNode is failing because this is failing: content->GetDocument(*getter_AddRefs(doc)); (content is the nsIContent that comes from a QI of the #text node inside the textarea). Kin -- what's the right test here?

Comment on attachment 120858 [details] [diff] [review] First step: don't intentionally skip them Whoops. I talked to Kin about this, and it turns out that the #text inside a textarea is only for the initial text; it doesn't change when the textarea is edited. So this patch doesn't help -- it's searching the wrong string. It turns out that there are two ways to do this: (1) Introduce a new interface that lets you go through the textarea's editor and get to the actual dom content, then use the normal find code on it. This is the preferred solution: it extends to work with midas, and it works with the existing find APIs. (2) Get textarea.value as a string, write separate search code to search through that, then somehow select the relevant part of the textarea using this interface: http://lxr.mozilla.org/seamonkey/source/dom/public/idl/html/nsIDOMNSHTMLInputEl ement.idl#53 Note that this doesn't translate to a range, so it can't work with the existing find interfaces, but it might work as a hackaround for people who have content management pages that use textareas.

*** Bug 210168 has been marked as a duplicate of this bug. ***

Welp, I'd love to see this one fixed sometime. It's something that bothers me in quite a few applications and Open since 2000 -- it's getting old. Makes me curious how many others are like this one.

any editor hacking still going on?

chofmann: yes; but unfortunately lower priority than finding another job position these days.

This is my number one request. Especially on the Mac this is important, because the only browser that can do this is IE. There are two problems with IE, though: (1) it is REALLY REALLY slow, and (2) MS stopped developing it and announced that there will be no new versions of it. In other words: it is no more.

Why is this bug labelled as an "enhancement"? The software doesn't do what it says it will do. What else could "Find in This Page" mean (see the Edit menu) but that the command will find the text if it is on the current page? Also, since we're coming up on the third anniversary of the filing of this bug (and I doubt it was the first to be filed for this issue, given the early comments along the lines of "Isn't this a dupe?"), wouldn't it be nice if we could have the poor thing emerge from its NEW status? (You know it's a bad sign when a bug's been around so long that the reporter is marked as "gone"!) The failure of the software to perform this basic function properly is crippling. Any chance the priority could be lifted out of the P5 basement?

Indeed, it's a bug needing fixing, not a desirable enhancement. Not comfortable in my new boots to change the P. Comment #31 suggests it may not be an easy fix.

It's an enhancement because the mozilla code never did this now please leave the field alone. the nextg change should be someone posting a patch or volunteering to fix this bug. if you change this bug then i will take your action as volunteering to fix this bug.

> It's an enhancement because the mozilla code never did this ... By this line of "reasoning" none of the security flaws which have been in IE from the start are bugs. :->} In case you hadn't noticed, the command does *not* say "Find in selected portions of this page." This bug is not about "parity with IE" but about the failure of the software to do what it says it will do. Have a nice time celebrating the bug's third anniversary!

Well said, couldn't agree more. > By this line of "reasoning" none of the security flaws which have > been in IE from the start are bugs. :->} > > In case you hadn't noticed, the command does *not* say "Find in > selected portions of this page." This bug is not about "parity > with IE" but about the failure of the software to do what it says > it will do. > > Have a nice time celebrating the bug's third anniversary!

*** Bug 222643 has been marked as a duplicate of this bug. ***

>In case you hadn't noticed, the command does *not* say "Find in >selected portions of this page." This bug is not about "parity >with IE" but about the failure of the software to do what it says >it will do. I faced problem when using Wikis. So this is a real disadvantage of the Firebird browser, you cannot search in edit field, for instance long wiki texts.

The problem is not only for wikis, but also for various other scripts allowing users to edit files over the web. This includes osCommerce where you can customize templates (this is how I came on this bug) It also includes just about any web-based file manager, such as the ones in geocities, and the like, and also the ones provided as part of a control panel on many shared hosting accounts. This means people editing any kind of HTML files over the web will have a much harder time using Mozilla based browsers than IE. I wish I had the knowledge to fix this myself, but I don't, and neither, I suspect, do many people whom this bug affects. This doesn't mean that it's an enchancement that should be left for the future. It is a flaw that cripples usability in a very broad scope of tasks that are common to the web. Whatever its marked severity and target milestone, this bug needs to be fixed, because it is one of those little annoyances that tends to drive people away.

*** Bug 228392 has been marked as a duplicate of this bug. ***

*** Bug 232671 has been marked as a duplicate of this bug. ***

My duplicate bug 222643 was marked as resolved. My focus was a little bit different. I DON'T want a ordinary full text search to process subwindows as well. I want an search option for edit field in the context menu, when the focus is on the edit field. You need it for wiki edit fields.

*** Bug 233841 has been marked as a duplicate of this bug. ***

*** Bug 202074 has been marked as a duplicate of this bug. ***

*** Bug 235598 has been marked as a duplicate of this bug. ***

Any news on this annoying bug, especially in connection with sites such as Wikipedia where searching within textareas is very important?

Well, it doesn't solve this bug, but maybe the searchtextarea bookmarklet I had made some time ago is of any use to you. http://home.hccnet.nl/m.wargers/bookmarklets/searchtextarea.htm

(In reply to comment #52) Thank you Martijn!!! Your javascript search works flawlessly within TWiki. If you weren't a dude, I'd propose... I'm so happy. But as you are, I'll just offer you my first born in gratitude. As much as I love Mozilla browsers, I've been so frustrated by this particular issue for years now. Maybe we all should pool our resources together and offer the first developer to fix this problem properly a round trip to Nevada's Bunny Ranch...

This item goes into the "ridiculous" category. That is, it's ridiculous that the comments go back to the year 2000 yet nothing has been done about it. Not only does this make Mozilla look bad, it also makes Wikis look bad. P5 priority? "Enhancement"? Nay, friends -- this is a critical bug.

(In reply to comment #54) > This item goes into the "ridiculous" category. > > That is, it's ridiculous that the comments go back to the year 2000 yet nothing > has been done about it. Stop ranting and start coding please; the most ridiculous here is your attitude. If this bug is 3 years old, it either means that 1. we have MUCH more important bugs on our radar 2. we still don't know how to implement it 3. the bug owner is no longer active on Mozilla > Not only does this make Mozilla look bad, it also makes Wikis look bad. Again, stop ranting and please contribute code if you really want to see this done ASAP. > P5 priority? "Enhancement"? Nay, friends -- this is a critical bug. Certainly not. What means "critical" is defined by http://bugzilla.mozilla.org/bug_status.html#severity In other words, RTFM.

I wonder if it would be possible to take comment #52: http://bugzilla.mozilla.org/show_bug.cgi?id=58305#c52 and use that (or similar) javascript in a popup XUL addition for alternative replace code? Could just add an option to the existing "Find" searchbox? That is could it be added to chrome://content/inspector/viewers/dom/FindDialog.js ? A nasty hack, but it may be a way to do it. I'm still learning this, so am not entirely sure....

anyone have thoughts, comments, or testing results on the suggestion above? run out of time for this in 1.7b. might consider for 1.7 final if we think the work around is a good idea, and has no side affects.

>anyone have thoughts, comments, or testing results on the suggestion above? Well, my code is a mess, so that would have to be rewritten. I am not able to get the precise position of the selection when the text wraps (the hard returns I can count but not the soft ones). So somehow that should be fixed, otherwise in situations with lots of soft wraps, the selection is not scrolled to the right position. Maybe an intermediate solution (which I think is easier to do) is when the focus is on the textarea and you press ctrl-F, you get a special search and replace (which would be handy and is different from the regular ctrl-F) for that textarea. Maybe I have an idea what causes this bug, maybe it is useful: The find dialog uses range objects to select the text. In a textarea this doesn't work very well, the text from the textNode inside the textarea element is selected, but that doesn't become visible on screen. I think that's the reason why they skip the textarea all together. Besides, the textnode in the textarea is not updated when you type some text in it, only textarea.value.

So this bug is about ctrl-f in an text area doesn't search that text area?

(In reply to comment #59) > So this bug is about ctrl-f in an text area doesn't search that text area? Yes/no. It's that using the find function in mozilla doesn't search a text area period. Whether you initiated the search from within the text area or from the whole page. In contrast I don't know of any other browser that doesn't search text areas when you search a page. I know IE does it, I'm pretty sure Opera does it, I know Safari does it. Firefox/Seafox don't.

not going to block the release for this feature. A well tested patch with full reviews and minimal or no impact on localizable strings would be considered.

doron: this bug is about the fact that find is based on ranges, as is selection, but there's no way to get the contents of a textarea as a range so there's no way to call find on it or to select the result if we did a find (see my comment 31). Until there's a way to do that, the discussion has concerned alternative workarounds like accel-F for a special textarea find that doesn't need ranges.

Code just posted to (not-quite-) duplicate Bug 120568.

*** Bug 238461 has been marked as a duplicate of this bug. ***

Well I made an extension for firefox, that popups a findtextareadialog when you press ctr-f in a textarea. It is still very basic, I have not changed much from the original finddialog.xul yet. I'm still learning all this xul/xbl stuff. I've found an (ugly, but pretty reliable working) workaround to get to scroll to the right position of the selection in the textarea. But would it still be useful to provide an extension, since I see in comment 63 that there is some kind of patch?

The code posted for Bug 120568 is not a patch, it's a temporary non-working changes made in (hopefully) progress to fixing this bug.

-> re-assiging to myself. I am going to take a stab at this but. As this bug is already too long, please do not post any non-technical comments (or comments about JS extensions, etc) while I own the bug. I am aiming at the fully-fledged version in the core code in parity with IE. That is, make the standard Find penetrate in a natural way into the text area/input control from the C++ side.

- the patch add supports for find in <textarea> and text <input> elements @see documentation at the top of nsFind.cpp for how this is done. This enables the editor to search & replace in the <HTML> source mode. - fix an old bug where the mutual exclusion of the selection wasn't respected. You can see this bug with a current Mozilla build on this bugzilla page for example: 1. select something outside a text input field. 2. select something in a text input field. -> bug: the selection isn't cleared outside @see documentation in nsTextControlFrame in the patch for the fix.

Comment on attachment 145567 [details] [diff] [review] proposed patch Asking akkana for review since this is her code that I have been munging.

same functionality as the previous patch, but with some factoring of code in nsFindContentIterator::First(), ::Last(), ::Reset() to make them more compact.

Oops... I meant nsFindContentIterator::Next() ::Prev, ::Reset().

A further simplification. I removed the mIsPositioned variable. It is redundant with the test of mInnerIterator. If you read the code: a non-null mInnerIterator means that mOuterIterator is positioned (per the logic in the patch).

Some further simplifications and bullet-proofing... akkana, do well to consider this one instead.

Correct a blunder in the Reset() function. The patch is nearing completion. I don't anticipate much improvements now other than corrections that may be discovered. To maximize the chances of getting approval for 1.7f (which is going to be a _long_ live branch), it will be good to see some testing by those of you on the CC list who build mozilla and were interested in this bug. Do well to report your testings to motivate drivers for approval.

As far as I can tell/test/think through again, I am now done with this gem, except this left-over in nsFindContentIterator::Reset() + mRange->GetStartContainer(getter_AddRefs(endNode)); + mRange->GetEndContainer(getter_AddRefs(endNode)); I am simply going to remove the first useless line at checkin (already done in my tree). The patch is now as compact as I would like, and as a side benefit it also fixes bug 189039 out-of-the-box.

Comment on attachment 145689 [details] [diff] [review] patch - take 5 Some initial comments (I haven't tested this yet): First, thanks! This looks like exactly the right way to solve the problem. It's nicely commented, too, and clear. Most of my comments that follow are questions about how it works, not criticism. There are a few lines going over 80 columns -- I'd prefer that you keep to the existing 80 column limit if possible. It doesn't build for me as is: | nsFind.cpp:56:23: nsIEditor.h: No such file or directory | nsFind.cpp:57:32: nsIPlaintextEditor.h: No such file or directory You need to add "editor" to the REQUIRES list (after which the build succeeds). Will everybody be okay with making find depend on editor? I don't have any problem with it myself. >Index: embedding/components/find/src/nsFind.cpp >+// 4) As of consequence of searching through text controls, we can be Possible typo, "as a consequence of"? (Your call.) >+ virtual nsresult Init(nsIDOMRange* aRange); Maybe a comment here about what aRange is? The range over which the iterator will operate, presumably? In Next() and Prev(): >+ MaybeSetupInnerIterator(); >+ if (mInnerIterator) { >+ mOuterIterator->First(); >+ mInnerIterator->First(); >+ } Why does mOuterIterator go back to First() here? I'm guessing it's for the same reason you call mOuterIterator->Init(range); in SetupInnerIterator() -- when you're done with the inner iterator then you set the outer iterator again? In Reset(): >+ for ( ; startContent; startContent = startContent->GetParent()) { >+ if (!startContent->IsNativeAnonymous()) { >+ mStartOuterNode = do_QueryInterface(startContent); >+ break; >+ } >+ } What does this do? >+ if (!mFindBackward) { >+ if (mStartOuterNode != startNode) { >+ SetupInnerIterator(startContent); >+ } >+ } >+ else if (mEndOuterNode != endNode) { >+ SetupInnerIterator(endContent); >+ } Why not MaybeSetupInnerIterator? I'm not clear on when you need Maybe and when you don't. >+nsresult >+NS_NewFindContentIterator(nsIContentIterator* aOuterIterator, >+ PRBool aFindBackward, >+ nsIContentIterator** aResult) I'm not clear on why you have to create a content iterator then pass it in to get a find content iterator. No way to create one in one step? [to be continued]

Comment on attachment 145689 [details] [diff] [review] patch - take 5 >Index: embedding/components/find/src/nsWebBrowserFind.cpp >=================================================================== [ ... ] >+// Same as the tail-end of nsEventStateManager::FocusElementButNotDocument. >+// Used here because nsEventStateManager::MoveFocusToCaret() doesn't >+// support text input controls. >+static void >+FocusElementButNotDocument(nsIDocument* aDocument, nsIContent* aContent) I'm not very familiar with this code (Aaron, do you know it?) Can code be shared between nsEventStateManager::FocusElementButNotDocument and this routine? Is there any chance that making the ESM support text controls (either optionally or all the time) would be desirable? The rest of nsWebBrowserfind.* look fine, at least from a quick reading. >Index: layout/html/forms/src/nsTextControlFrame.cpp >=================================================================== >>+#if 0 // moved to SetFocus() > // first, tell the caret which selection to use > nsCOMPtr<nsISelection> domSel; > GetSelection(nsISelectionController::SELECTION_NORMAL, getter_AddRefs(domSel)); >@@ -595,6 +596,7 @@ > shell->GetCaret(getter_AddRefs(caret)); > if (!caret) return NS_OK; > caret->SetCaretDOMSelection(domSel); >+#endif Why leave the dead code there? Better to remove it entirely. Aaron, can you comment on the changes to this file? Is it okay to move this code from SetCaretEnabled() to SetFocus()? I'll do some testing of the patch over the next few days. I hope others who are interested can also help with testing.

> There are a few lines going over 80 columns -- I'd prefer that you keep to the > existing 80 column limit if possible. Will do. > > It doesn't build for me as is: > | nsFind.cpp:56:23: nsIEditor.h: No such file or directory > | nsFind.cpp:57:32: nsIPlaintextEditor.h: No such file or directory > > You need to add "editor" to the REQUIRES list (after which the build > succeeds). > Will everybody be okay with making find depend on editor? I don't have any > problem with it myself. Apparently, the REQUIRES changes that I made didn't show up on the patch. Strange that the |Makefile|s are excluded from cvs diff? I don't have a strong feeling either way regarding the dependency. Some public methods of nsITextControlFrame actually drag in the editor (ender). > >>Index: embedding/components/find/src/nsFind.cpp >>+// 4) As of consequence of searching through text controls, we can be > > > Possible typo, "as a consequence of"? (Your call.) Corrected. >>+ virtual nsresult Init(nsIDOMRange* aRange); > > Maybe a comment here about what aRange is? The range over which the iterator > will operate, presumably? Yes, that's what it is, but... it operates in a segmented manner (see below). > In Next() and Prev(): > >>+ MaybeSetupInnerIterator(); >>+ if (mInnerIterator) { >>+ mOuterIterator->First(); >>+ mInnerIterator->First(); >>+ } > > Why does mOuterIterator go back to First() here? I'm guessing it's for the > same reason you call mOuterIterator->Init(range); in SetupInnerIterator() -- > when you're done with the inner iterator then you set the outer iterator > again? This goes to the nub of the idea behind the patch, and as I will explain, it is why it works reliably. When an mInnerIterator is created, the mOuterIterator is re-inited with the remaining _segment_ of mRange which is yet to be visited (picture the segment rightward in find-forward and leftward in find-backward). It is a total re-init. Thus mOuterIterator has to be put at First() or at Last() accordingly. Its re-init() happens early in SetupInnerIterator, but First()/Last() happen later when it becomes known that we are in the context of Next()/Prev(). The nett result of this process is to put mOuterIterator on the node after (or before) the <textarea>. It is more readable and less error-prone that trying to loop pass a "<textarea>...big markup here...</textarea>", which might be the last/first element. Thus we use a robust way to examine the live text in the editor while skipping the initial text, including edge cases where only a part of the text control has to be examined. > In Reset(): > >>+ for ( ; startContent; startContent = startContent->GetParent()) { >>+ if (!startContent->IsNativeAnonymous()) { >>+ mStartOuterNode = do_QueryInterface(startContent); >>+ break; >>+ } >>+ } > > What does this do? This is what I commented in point 4) in the header. If the current selection is inside a text control, the starting point is the corresponding (anonymous) text node. Therefore the for-loop above will walk up, all the way to the (non-anonymous) <textarea> node itself and use it as the reference outer-point. With WRAPPING on, find happens from SelEnd to DocEnd and then DocStart to SelStart. Hence it is possible for the anonymous text node to be either a starting point or an ending point. That's why Reset() takes no chance and deals with both situtations. Also, we can hit either point later as we iterate in the find, and that's why there is a check again later in SetupInnerIterator. >>+ if (!mFindBackward) { >>+ if (mStartOuterNode != startNode) { >>+ SetupInnerIterator(startContent); >>+ } >>+ } >>+ else if (mEndOuterNode != endNode) { >>+ SetupInnerIterator(endContent); >>+ } > > Why not MaybeSetupInnerIterator? I'm not clear on when you need Maybe and > when you don't. If there is no anonymous text node (continuing with my clarifications above), we will get mStartOuterNode == startNode. Otherwise, there _is_ a <textarea> (or text <input>). It is not "maybe" anymore, and we can go straight to business, without having to depend, at this point, on mOuterIterator->GetCurrentNode(). >>+nsresult >>+NS_NewFindContentIterator(nsIContentIterator* aOuterIterator, >>+ PRBool aFindBackward, >>+ nsIContentIterator** aResult) > > I'm not clear on why you have to create a content iterator then pass it in to > get a find content iterator. No way to create one in one step? Possible. It crossed my mind. But for the sake of readability, and with the business of segmented ranges, I prefer to have, in effect, three iterators. The main wrapper for the outside world, then those inner- and outer- helpers that change dynamically. [If the outer one is the main one, looping pass a <textarea> would be lot messier.] Should I |new| the outer-iterator inside or outside? That was another question. I simply opted to re-use the code with its documentation, and see what reviewers think. I can move that inside FindContentIterator::Init() if you like. > Is there any chance that making the ESM support text controls > (either optionally or all the time) would be desirable? I actually tried to track the actual caret's DOM selection in the ESM, but everybody and their friends there assume that it comes from the pres shell. It was so much hassle and riskier at this 1.7f stage than replicating FocusElementButNotDocument(). > Why leave the dead code there? Better to remove it entirely. Will do. >Aaron, can you comment on the changes to this file? Is it okay to >move this code from SetCaretEnabled() to SetFocus()? Actually setting the caret's DOM selection in SetCaretEnabled() was considered a nasty side-effect in the pres shell. http://lxr.mozilla.org/mozilla/source/layout/html/base/src/nsPresShell.cpp#3096 It seems more natural to set it (by default) when the element has focus, while leaving other non default settings to callers that explicitly want to do it.

rbs: you need to change Makefile.in

- add editor in REQUIRES - more comments on how it works - hide the outer-iterator from the outside world - limit to 80ch - remove dead code

Comment on attachment 145985 [details] [diff] [review] patch - updated per review comments Sorry, this is a large patch and I don't have time to superreview.

Comment on attachment 145985 [details] [diff] [review] patch - updated per review comments r=akkana The changes all look good. I like hiding the outer iterator from the outside world in the creator ... seems cleaner. I haven't done a lot of testing, but this works on textareas forward and backward, and it still passes all the Find regression tests at http://www.mozilla.org/quality/browser/front-end/testcases/xpapps-gui/find-on-p age/find-in-page.html I saw one weirdness: if you use find-in-page for a string that is found in a textarea, the string is not highlighted, though it is correctly found. With normal find, from the dialog, the string is highlighted; and find-in-page highlights strings outside of textareas. Not a big problem -- scrolling happens correctly and the caret ends up at the right place, which are the important things. If JST doesn't have time, Kin would be a good super-reviewer.

From comment 82 , that would be bug 134586 I think. (Great job by the way, rbs)

> From comment 82 , that would be bug 134586 I think. It looks like that's indeed the problem, and it is Unix-specific. I wasn't even aware of that difference because the find's selection in the textarea appears fine on my Win2K box (the correct scrolling is indicative that it is a paint problem due to that reported Unix paint difference). I wish bug 134586 could be fixed too because Unix users will miss out on the visual cue that the visible selection provides for the find.

akkana, martijn (if you have a ready build with the patch), do you mind trying the following experiment in nsWebBrowserFind::SetSelectionAndScroll (two changes) and see if it makes any difference?!? if (selection) { selection->RemoveAllRanges(); -//OLD: commented selection->AddRange(aRange); // Scroll if necessary to make the selection visible: selCon->ScrollSelectionIntoView (nsISelectionController::SELECTION_NORMAL, nsISelectionController::SELECTION_FOCUS_REGION, PR_TRUE); if (tcFrame) +{ FocusElementButNotDocument(doc, content); +// NEW: focus _before_ selecting selection->AddRange(aRange); +} else { +// keep old behavior for normal find + selection->AddRange(aRange); nsCOMPtr<nsIPresContext> presContext; presShell->GetPresContext(getter_AddRefs(presContext)); PRBool isSelectionWithFocus; presContext->EventStateManager()-> MoveFocusToCaret(PR_TRUE, &isSelectionWithFocus); } }

Oops... the suggested change is not quite correct. Scroll is misplaced -- it should happen after adding the range.

Sorry, I'm leaving town today and won't have net access for a week or two ... maybe martijn can test it.

>I saw one weirdness: if you use find-in-page for a string that is found in a ^^^^^^^^^^^^ >textarea, the string is not highlighted, though it is correctly found. With >normal find, from the dialog, the string is highlighted; ^^^^^^^^^^^ It just occurred to me that you may be meaning 'find-as-you-type'... and that all is well with "normal find" (what we actually know as find-in-page). If that is so, yes I saw that. The matches from 'find-as-you-type' are not highlighted. But the scrolling and cursor work fine. It is a really a tagent from the initial goal. I think it is something that aaronl can look at later. My patch is clearly doing a selection, but his code seems to clear that in order to make it green (as find-as-you-type does), but end up not doing it. But even without the selection, the visible cursor (in the textarea) gives a cue in that very specific case.

*** Bug 240488 has been marked as a duplicate of this bug. ***

Yes, oops, I meant find-as-you-type, and yes, bug 134586 sounds like it.

Comment on attachment 145985 [details] [diff] [review] patch - updated per review comments sr=jst

Comment on attachment 145985 [details] [diff] [review] patch - updated per review comments checked into the trunk. Asking approval for 1.7 as this is a much sought-after feature that stops IE users from fully migrating to Gecko.

This broke the tree due to changes that don't seem to be in the most recent patch on this bug. I emailed rbs, but my email bounced: Final-Recipient: rfc822; <rbs@maths.uq.edu.au> Action: failed Status: 5.1.0 MAIL FROM: <dbaron@dbaron.org> 550 REPLY: 550_5.7.1_Access_denied Diagnostic-Code: smtp; Permanent Failure: Other address status Last-Attempt-Date: Thu, 15 Apr 2004 20:18:56 -0000

The bustage has nothing to do with REQUIRES as rbs indicated on tinderbox. The problem is that there's no |mFindForward| variable.

(And in the future, it would be much easier if you were on irc.mozilla.org #developers when you break the tree, especially since your email doesn't work.)

Thanks for the follow-up, dbaron, the bustage came from a typo on a last minute extra. I just checked in a fix. (I don't know why my e-mail bounced, BTW. I cannot be on IRC. Our local policy denies such access.)

Why is this bug left open if the fix was checked in ?

Marking FIXED. The wait for approval for 1.7f doesn't depend on that indeed.

Looks like bug 120568 is a dupe of this one, isn't it?

Bug 120568 wants Find and Replace, not just Find.

rbs: I've also had mail bounce trying to mail you directly. The message was: <rbs@maths.uq.edu.au>: host mailhub2.uq.edu.au[130.102.5.59] said: 550 5.7.1 Access denied Perhaps an overly aggressive spam filter? (Sorry to spam this bug, but obviously neither of us can mail rbs with the error msg ...)

(In reply to comment #100) > Bug 120568 wants Find and Replace, not just Find. It took three years to get find. Now they want find and replace? Yeeeesh. Not much more to add to get that. But I see problems. Basically we have to distinguish if the element found is within a form. Not to mention if it's in the correct form that the user wanted the replace to occurr. Although... I was thinking about whether I'd want just the same type of thing myself. Is Mozilla a text editor? ;)

*** Bug 241645 has been marked as a duplicate of this bug. ***

Comment on attachment 145985 [details] [diff] [review] patch - updated per review comments a=asa (on behalf of drivers) for checkin to 1.7

-> Now FIXED on the 1.7 branch.

*** Bug 243423 has been marked as a duplicate of this bug. ***

Now find-as-you-type seems to be searching in text fields, but the text isn't highlighted at all. This is very confusing to the user.

I think there are some other weirdnesses with the way it works with Find As You Type. For example, when FAYT is finished (via Escape to cancel or timeout), the focus goes outside the textarea even if that's where the text was found. You should be in the text area with the text selected at that point.

Please use bug 189039 for the remaining specific issues for find-as-you-type. I think FAYT needs to take into account the _new_ fact that matches can be inside textareas. As this bug has shown, things are little bit different with text boxes. I can assist in bug 189039 if you need clarifications in certain points. In particular (re: comment 107 and comment 108), to make the selection green (as FAYT does) or focus, FAYT needs to be careful with two things: - make sure to sync the primary frame as the text control, as appopriate - make sure to use the LOCAL selection inside the textarea, as appropiate. [I looked at the code of FAYT when I was working on this bug and FAYT was only paying attention at the GLOBAL document selection.]

verified

Can we have this reopened for firefox? Since it's not fixed there?... Affects: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20041218 Firefox/1.0+ Unless there is something wrong with the build I'm using....

(In reply to comment #111) > Can we have this reopened for firefox? No, since this is not a Firefox bug, bug 252371 is

*** Bug 152299 has been marked as a duplicate of this bug. ***