Bug 9456 – REGRESSION (417.9.2-420+): Cmd-G doesn't work in Safari after double-click and Cmd-E

• Status: RESOLVED
• Resolution: FIXED
• Component: Text

[John Sullivan]

This bug is also in Radar as <rdar://4587986>

To reproduce:

1. Visit any page that contains a word that occurs more than once, e.g. http://www.amazon.com
2. Double-click such a word (e.g. "the")
3. Cmd-E to put the word on the Find pasteboard
4. Cmd-G to select and reveal the next occurrence

The next occurrence isn't found; the selection remains on the word double-clicked in step 1.

Note that dragging across the word instead of double-clicking it does not cause this bug to occur, so there's something special about double-clicking here.

Also note that if you click outside of the selection after step 3, then Cmd-G will correctly cycle around all occurrences of the word including the first one. So it seems that the text put on the Find pasteboard is correct.

-------------------------------------------

[John Sullivan]

The problem is in Frame::findString() -- there's a test to see whether the result range after calling findPlainText() matches the current selection. If so, the code looks for the next occurrence. But when this bug occurs, the code thinks the result range does not match the selection, so the already-selected text is left selected.

This code has not changed recently, and I'm pretty sure that this bug is pretty new. So it seems that the problem is that the Selection computed after double-clicking a word no longer matches the Selection computed after (e.g.) dragging across the same word. Maybe the definition of the "=" operator changed for Selection? Maybe double-clicking is now making the Selection object different in some way than it used to?

[John Sullivan]

I was wrong -- this code has actually changed recently:

2006-06-09  Justin Garcia  <[e-mail]>

        Reviewed by levi
        
        <rdar://problem/4549980>
        REGRESSION: "Find Again" can get stuck when searching for string with a trailing space

        <...>
        * page/Frame.cpp:
        (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.


I strongly suspect that this change was the cause of this regression. I'm reassigning to Justin since he knows this Selection stuff well and he made that recent change.

[Justin Garcia]

Created attachment 8860 [details]
patch

[Justin Garcia]

Comment on attachment 8860 [details]
patch

dave reviewed this

[David Harrison]

Comment on attachment 8860 [details]
patch

r=me

[Justin Garcia]

r14877