Closed Bug 241053 Opened 19 years ago Closed 19 years ago

find in textarea misses some matches

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7final

People

(Reporter: rbs, Assigned: rbs)

Details

(Keywords: fixed1.7)

Attachments

(2 files)

This is due to a glitch in nsFindContentIterator::Reset().
If you have been following bug 58305, you are probably aware of my wrestling
with the edge cases of that function. I will attach a patch to correct the problem.
Attached patch fixSplinter Review
Some clean-ups:
- remove some further dead code that I discovered later in nsWebBrowserFind.cpp

- don't bother re-creating mOuterIterator when not necessary in Init()
- move the gimmicks of mOuterIterator->First() [or mOuterIterator->Last] just
after |SetupInnerIterator| inside |MaybeSetupInnerIterator| and |Reset|. This
way, those gimmicks don't have to be spread at caller sites.

The actual fix to this bug (in |Reset|): 
- the direct call to |SetupInnerIterator| in Reset() was based on an if-clause
that misses the case where the boundary point _is_ a text control. The clause
was only capturing the case where the point is inside the text control. The fix
 is to continue to try to create the inner-iterator if it wasn't created
already.
Comment on attachment 146570 [details] [diff] [review]
fix

Should drivers approve bug 58305 for 1.7f, this fix up will be necessary as
well.

sicking, do you mind reviewing this? akkana did the earlier review, but she is
on vacation at present.

jst, this is a small continuation of the earlier bug.

Thanks.
Attachment #146570 - Flags: superreview?(jst)
Attachment #146570 - Flags: review?(bugmail)
Comment on attachment 146570 [details] [diff] [review]
fix

sr=jst
Attachment #146570 - Flags: superreview?(jst) → superreview+
For my sanity sake, I am also going to stabilize PositionAt() as indicated
below. I have discovered that if you do PositionAt(PARIS), the iterator code
will put its current node there before returning failure. And so you can't do
much else with it, unless you stash the current node away first.

nsresult
nsFindContentIterator::PositionAt(nsIContent* aCurNode)
{
  nsIContent* oldNode = mOuterIterator->GetCurrentNode();
  nsresult rv = mOuterIterator->PositionAt(aCurNode);
  if (NS_SUCCEEDED(rv)) {
    MaybeSetupInnerIterator();
  }
  else {
    mOuterIterator->PositionAt(oldNode);
    if (mInnerIterator)
      rv = mInnerIterator->PositionAt(aCurNode);
  }
  return rv;
}
Comment on attachment 146570 [details] [diff] [review]
fix

Trying akkana since sicking is apparently unwilling or unable to attend to
this.

akkana, based on your earlier review, this is a simple follow-up that shouldn't
take much to look at.
Attachment #146570 - Flags: review?(bugmail) → review?(akkzilla)
Comment on attachment 146570 [details] [diff] [review]
fix

r=akkana
Attachment #146570 - Flags: review?(akkzilla) → review+
Comment on attachment 146570 [details] [diff] [review]
fix

checked in the trunk.

drivers, this is a follow-up to bug 58305 for which the approval is also
pending.
Attachment #146570 - Flags: approval1.7?
-> FIXED on the 1.8a trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee: guifeatures → rbs
Status: REOPENED → NEW
Status: NEW → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
Comment on attachment 146570 [details] [diff] [review]
fix

a=asa (on behalf of drivers) for checkin to 1.7
Attachment #146570 - Flags: approval1.7? → approval1.7+
-> Now FIXED on the 1.7 branch.
Keywords: fixed1.7
Target Milestone: --- → mozilla1.7final
Product: Core → Mozilla Application Suite
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.