Bug 12675 – REGRESSION: onBlur doesn't fire when focus switches between subframes

• Component: Frames
• Status: RESOLVED FIXED

[Maciej Stachowiak]

2006-12-10 15:22:08 Adam Roben:
* SUMMARY
We don't dispatch blur events when switching between subframes.

* STEPS TO REPRODUCE
1. Save blur.html and blurFrame.html to the same folder
2. Open blurFrame.html in ToT Safari
3. Type in the text field on the left
4. Type in the text field on the right

* RESULTS
No blur event is fired. If the event is fired, an alert dialog will pop up.

2006-12-12 19:37:07 Adam Roben:
Assigning to Geoff since he's working on focus issues.

2006-12-18 13:48:08 Stephanie Lewis:
Safari BRB Reviewed

2007-01-15 14:10:29 Alice Liu:
Safari blocker reviewed

<rdar://problem/4874825>

[Maciej Stachowiak]

Created attachment 13282 [details]
blur.html

[Maciej Stachowiak]

Created attachment 13283 [details]
blurFrame.html

[Dex Deacon]

I don't have permission to assign bugs, but I am looking into this.

[Dex Deacon]

Created attachment 13415 [details]
Use FocusController::setFocusedNode in the EventHandler

I had to modify an existing layout test (fast/frames/iframe-window-focus.html) to pass under this change.  I think it's a valid modification though.  The test was blurring and then re-focusing a text node after entering some text.  It then entered more text, assuming the cursor position was preserved.

This worked before because the node never actually received the blur event.  With my change, however, the cursor position is reset when the node is blurred.

[Maciej Stachowiak]

I thought we had code to intentionally restore caret position when focusing a text field via script. I could be remembering wrong though. I think Adele knows the most about this.

[David Kilzer (:ddkilzer)]

Adding Adele to CC list per Comment #5.

[Adele Peterson]

Comment on attachment 13415 [details]
Use FocusController::setFocusedNode in the EventHandler

This seems fine to me.

Its weird though, I applied the patch in WebCore, and iframe-window-focus.html still passes for me.  Are you sure that change is needed?

[Adele Peterson]

Comment on attachment 13415 [details]
Use FocusController::setFocusedNode in the EventHandler

Just going to r- this until we figure out the layout test issue.

[Adele Peterson]

Something was wrong with my setup before.  I'm not sure what, but now I can see the layout test difference.

Here's the thing, this does expose a behavior problem with rich text editing in GMail.

If you start typing, and then change the font color (using one of their buttons), the cursor gets set back to the beginning.

This is sort of a separate problem, but the symptom is pretty bad.  I'm tempted to make this bug blocking the new bug...that someone should file... about restoring a previous selection for editable elements.

[Dex Deacon]

In Document::setFocusedNode(), the call to clearSelectionIfNeeded() is what resets the cursor position.  It doesn't appear to be stored anywhere, so it would seem to be that case that unfocusing a node would always reset the cursor position.  Maybe the solution involves storing the selection (or at least part of it) somewhere before we clear it.

[Adele Peterson]

I think we can probably fix this by changing FocusController::setFocusedNode.  I think its clearing out the oldDocument's focusNode too aggressively.  I'm testing a change for that in my tree.

If that fixes this frame focus problem, we should be able to proceed with the original fix.

[Adele Peterson]

I meant, frame caret problem.

[Adele Peterson]

I think I was wrong in my last comment.  Since you need to clear the focusedNode to get blur events, we can't really get rid of any of the setFocusedNode(0) calls.

Dex, your idea about refining clearSelectionIfNeeded sounds good.  Ideally, I think we'll want to be able to save and restore selection for individual elements, but doing this at the frame level might be a good first step.

I might be talking myself in circles here.

[Adele Peterson]

This is fixed in TOT.  Probably by 21467.

[Adele Peterson]

I narrowed the range down to 21448 - 21494, and 21467 is my best guess