Fix the range returned by findFirstMisspelling

Fix the range returned by findFirstMisspelling

This fixes a regression made in [1].  The 'firstMisspellingRange' is supposed
to be the range of the misspelling, not the range of the text that was checked.

[1] https://codereview.chromium.org/576073003

BUG=
R=yosin@chromium.org

[yosin_UTC9, 2015-05-21 02:28:49]

Sorry for super late response. m(_ _)m

https://codereview.chromium.org/908303002/diff/1/Source/core/editing/TextChec...
File Source/core/editing/TextCheckingHelper.cpp (right):

https://codereview.chromium.org/908303002/diff/1/Source/core/editing/TextChec...
Source/core/editing/TextCheckingHelper.cpp:272: firstMisspellingRange =
Range::create(misspellingStart.containerNode()->document(), misspellingStart,
misspellingEnd);
It seems current code is correct, since |misspellingStart == m_start| and
|misspellingEnd == m_end|.

[Shezan Baig (Bloomberg), 2015-05-21 09:19:39]

On 2015/05/21 02:28:49, Yosi_UTC9 wrote:
> Sorry for super late response. m(_ _)m
> 
>
https://codereview.chromium.org/908303002/diff/1/Source/core/editing/TextChec...
> File Source/core/editing/TextCheckingHelper.cpp (right):
> 
>
https://codereview.chromium.org/908303002/diff/1/Source/core/editing/TextChec...
> Source/core/editing/TextCheckingHelper.cpp:272: firstMisspellingRange =
> Range::create(misspellingStart.containerNode()->document(), misspellingStart,
> misspellingEnd);
> It seems current code is correct, since |misspellingStart == m_start| and
> |misspellingEnd == m_end|.


It does look like that at first, doesn't it? :)

It took me a while to figure out, but they are actually not the same because
|misspellingStart| and |misspellingEnd| are modified by the
|TextIterator::subrange| function.

[yosin_UTC9, 2015-05-22 01:32:48]

On 2015/05/21 09:19:39, Shezan Baig (Bloomberg) wrote:
> On 2015/05/21 02:28:49, Yosi_UTC9 wrote:
> > Sorry for super late response. m(_ _)m
> > 
> >
>
https://codereview.chromium.org/908303002/diff/1/Source/core/editing/TextChec...
> > File Source/core/editing/TextCheckingHelper.cpp (right):
> > 
> >
>
https://codereview.chromium.org/908303002/diff/1/Source/core/editing/TextChec...
> > Source/core/editing/TextCheckingHelper.cpp:272: firstMisspellingRange =
> > Range::create(misspellingStart.containerNode()->document(),
misspellingStart,
> > misspellingEnd);
> > It seems current code is correct, since |misspellingStart == m_start| and
> > |misspellingEnd == m_end|.
> 
> 
> It does look like that at first, doesn't it? :)
> 
> It took me a while to figure out, but they are actually not the same because
> |misspellingStart| and |misspellingEnd| are modified by the
> |TextIterator::subrange| function.

Good catch! It is difficult to know that. Using non-const reference isn't good.
We should use pointer.

Could you write test script for your change? 
I think DocumentMarkerControllerTest.cpp is the best place.

Thanks!

[Shezan Baig (Bloomberg), 2015-05-22 11:51:05]

On 2015/05/22 01:32:48, Yosi_UTC9 wrote:
> Could you write test script for your change? 
> I think DocumentMarkerControllerTest.cpp is the best place.


Hmm, I looked at DocumentMarkerControllerTest.cpp but not sure what's the best
way to test this there.  The TextCheckingHelper needs a SpellCheckerClient, and
looks like the DummyPageHolder inside DocumentMarkerControllerTest.cpp doesn't
create a SpellCheckerClient.

I noticed that HTMLTextFormControlElementTest.cpp does have a
DummySpellCheckerClient.

So I wonder if HTMLTextFormControlElementTest.cpp would be a better place?  Or
do you think we should add a DummySpellCheckerClient to
DocumentMarkerControllerTest.cpp?

[yosin_UTC9, 2015-05-25 04:25:14]

On 2015/05/22 11:51:05, Shezan Baig (Bloomberg) wrote:
> So I wonder if HTMLTextFormControlElementTest.cpp would be a better place?  Or
> do you think we should add a DummySpellCheckerClient to
> DocumentMarkerControllerTest.cpp?

I prefer DocumentMarkerControllerTest.cpp to have test script in same directory.