Bug 4840 – Frames only resize wider, can't make them narrower

• Component: Frames
• Status: RESOLVED FIXED

[Douglas Livingstone]

1) Visit http://api.rubyonrails.com/
2) Drag the vertical divider to the right
3) Drag it back to the left

Expected:
Gets narrower

Actual:
Nothing happens

[Oliver Hunt]

confirming, needs a minimised testcase though

[Douglas Livingstone]

Created attachment 3794 [details]
3 frame testcase

[Alexey Proskuryakov]

*** Bug 5452 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

Reassigning to webkit-unassigned, to make sure more people see this. We need a separate P1 bug for the issue mentioned in bug 5452 comment 1...

Created attachment 6833 [details]
Don't lose track of the resizing frameset

[Please comment and r- anyway since this is missing a test and a change log entry].

This bug is caused by the fact that when the splitter is dragged to the left, the mouse moved (and ultimately mouse released) events are being sent to the inner frameset (on the left) instead of the currently resizing outer frameset.

The fix is to dispatch all mouse events to the currently-resizing frameset. There may be some way to make hitTest give the right answer, but I don't think RenderFrameSet::nodeAtPoint() can be modified in a helpful way.

[Darin Adler]

Comment on attachment 6833 [details]
Don't lose track of the resizing frameset

We would like a capturing mechanism that works for all drags. But adding one specifically for the frameset resizing seems fine for now.

It's not good to add stuff about resizing to the DOM, though. This should stay in the view system (rendering and FrameView).

+    DocumentImpl *d = getDocument();
+    FrameView *w = d ? d->view() : 0;
+    if (w)
+        w->setResizingFrameSet(e ? this : 0);

getDocument no longer ever returns 0 -- nodes now reference their document -- so we don't need a nil-check on the document. I'd also like to see the FrameView* style used instead of FrameView * as per our style guidelines. But this function should go.

RenderFrameSet::setResizing should handle this directly, calling the FrameView. You don't need a boolean in the renderer or the DOM element.

Nice fix in hoverTimerFired.

(In reply to comment #6)
> Nice fix in hoverTimerFired.

Oops! That part comes from a different patch (bug 7555) and doesn't belong here.

I'll redo the patch based on the other comments.

Created attachment 6853 [details]
Revised patch, including manual test and change log

[Darin Adler]

Comment on attachment 6853 [details]
Revised patch, including manual test and change log

r=me

[Darin Adler]

Comment on attachment 6853 [details]
Revised patch, including manual test and change log

r=me

[Alexey Proskuryakov]

Landed, r13135.