Remove code in Editor::rangeOfString() that resets the searchRange irrespective of findNext value.

Remove code in Editor::rangeOfString() that unnecessarily resets the searchRange after the first find operation.

Currently, Editor::RangeOfString() sets the start and end positions for the searchRange depending on the findOptions.findNext option's value. If 'StartInSelection' has been set (for findNext) then the start of the searchRange would point to the start of the current active (matched) selection.
Thus the search would begin from this point and the current active selection will be returned as the result/matched range in case the search keyword is same as for the previous find operation.

But, right after this resultRange is obtained, we have code (in rangeOfString()) that compares the new resultRange with the current active range and if found to be the same (which would be the case when the search keyword remains the same), it resets the start and end positions of the searchRange, this time from after the current active selection, irrespective of whatever the findOptions.findNext value is set to, and then performs another find operation to get a new resultRange. This new resultRange (if found) would thus lie beyond the previous active (matched) selection.

This results in erroneous behavior for scenarios wherein, even though the user/application doesn't move to the next match, simply performing another find operation using the same keyword would unnecessarily increment the activeMatch range as well as count.

I found this code to have been added in the patch: http://trac.webkit.org/changeset/14782. However, the changelog just states: "(WebCore::Frame::findString): Moved from FrameMac. Compare a selection created using the found range with the current selection in case the current selection is the found range minus some collapsed whitespace on the edges."

This change should not cause any of the existing tests (layout as well as unit tests) to fail.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=160178

[tkent, 2013-10-22 06:49:39]

> Currently, Editor::RangeOfString() sets the start and end positions for the
searchRange depending on the 

The first line of the CL description should be a short summary of the CL.
It should be "Remove blah blah of Editor::RangeOfString()" in this case.

[arpitab_, 2013-10-22 06:56:31]

On 2013/10/22 06:49:39, tkent wrote:
> > Currently, Editor::RangeOfString() sets the start and end positions for the
> searchRange depending on the 
> 
> The first line of the CL description should be a short summary of the CL.
> It should be "Remove blah blah of Editor::RangeOfString()" in this case.

Edited the subject to "Remove code in Editor::rangeOfString() that resets the
searchRange irrespective of findNext value."
Hope that is more appropriate.

[tkent, 2013-10-22 06:59:55]

On 2013/10/22 06:56:31, arpitab_ wrote:
> On 2013/10/22 06:49:39, tkent wrote:
> > > Currently, Editor::RangeOfString() sets the start and end positions for
the
> > searchRange depending on the 
> > 
> > The first line of the CL description should be a short summary of the CL.
> > It should be "Remove blah blah of Editor::RangeOfString()" in this case.
> 
> Edited the subject to "Remove code in Editor::rangeOfString() that resets the
> searchRange irrespective of findNext value."
> Hope that is more appropriate.

I didn't mean the subject of this codereview issue, which won't be used for
actual commit.
I meant the fist line of the CL description literally.

[arpitab_, 2013-10-22 07:04:53]

On 2013/10/22 06:59:55, tkent wrote:
> On 2013/10/22 06:56:31, arpitab_ wrote:
> > On 2013/10/22 06:49:39, tkent wrote:
> > > > Currently, Editor::RangeOfString() sets the start and end positions for
> the
> > > searchRange depending on the 
> > > 
> > > The first line of the CL description should be a short summary of the CL.
> > > It should be "Remove blah blah of Editor::RangeOfString()" in this case.
> > 
> > Edited the subject to "Remove code in Editor::rangeOfString() that resets
the
> > searchRange irrespective of findNext value."
> > Hope that is more appropriate.
> 
> I didn't mean the subject of this codereview issue, which won't be used for
> actual commit.
> I meant the fist line of the CL description literally.

Oops! Sorry about that! Have modified the CL description now.

[tkent, 2013-10-22 07:08:43]

lgtm

rubber-stamping

[commit-bot: I haz the power, 2013-10-22 07:09:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/a.bah@samsung.com/34503002/1

[commit-bot: I haz the power, 2013-10-22 10:31:38]

Change committed as 160178