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>
Created attachment 13282 [details] blur.html
Created attachment 13283 [details] blurFrame.html
I don't have permission to assign bugs, but I am looking into this.
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.
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.
Adding Adele to CC list per Comment #5.
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?
Comment on attachment 13415 [details] Use FocusController::setFocusedNode in the EventHandler Just going to r- this until we figure out the layout test issue.
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.
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.
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.
I meant, frame caret problem.
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.
This is fixed in TOT. Probably by 21467.
I narrowed the range down to 21448 - 21494, and 21467 is my best guess