579767 - Frame borders are sometimes not redrawn correctly after dragging them or resizing the window

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Andreas Jung]

User-Agent:       Mozilla/5.0 (Windows; Windows NT 5.1; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre
Build Identifier: Mozilla/5.0 (Windows; Windows NT 5.1; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre

First appeared in:
Mozilla/5.0 (Windows; Windows NT 5.1; en-US; rv:2.0b2pre) Gecko/20100716 Minefield/4.0b2pre 

Probably caused by bug 564991

Reproducible: Always

Steps to Reproduce:
1. Open the testcase in the URL field
2. Move/Drag the frames' borders around

Actual Results:  
The borders are sometimes not redrawn properly.

Expected Results:  
The borders are drawn correctly.

Comment 1

[Andreas Jung]

Attachment: Screencast showing the bug

Comment 2

[Andreas Jung]

Another way to reproduce this bug:
1. Open the testcase in the URL field
2. Resize the window

Comment 3

[Joe Drew (not getting mail)]

Does this still happen?

Comment 4

[Jim Mathies [:jimm]]

(In reply to comment #3)
> Does this still happen?

Yes, pretty minor. Best way to get it is to:

1) open test case
2) drag the vertical line out a few inches
3) drag the horizontal line down an inch while pulling the mouse to the right a bit.

You'll get a corrupt vertical line sometimes.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yeah, I can see it now.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I see this on Linux too.

I wonder if the display items for the frameset borders are reporting the correct bounds.  (I presume they have their own display items, though maybe not.)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

They do, but a cursory code inspection looks OK.

Comment 8

[(no longer active)]

This is, in fact, not a regression from retained layers.  It also happens on the 2010-07-15 nightly.

Comment 9

[(no longer active)]

OK, here's the source of the bug.

When we're scrolling the vertical border to the right, we set mColSizes which is in app units here: <http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrameSetFrame.cpp#1470>, and then we set the cols attribute on the content node based on the result.  We convert the dimensions into percentage values here: <http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrameSetFrame.cpp#630>.

At the second stage of the test, when dragging down the horizontal border, we get a reflow on the topmost frameset element, which brings us here: <http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsFrameSetFrame.cpp#988>, which causes us to attempt to calculate the app unit values from the percentage values, which causes the vertical border to "jump" a little bit to the right.  The thing to note here is that mDrag.mActive is false, because the drag operation is happening on a child frameset, so we redo the width calculation when we don't really have to.

This is actually a very old behavior which has been broken for a long time.  Do we *really* want to block 2.0 on this?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

No. But we'd take a patch now that you've debugged it! :-)

Comment 11

[(no longer active)]

Attachment: Patch (v1)

After playing with this a bit more, I finally have a patch which I'm happy with.  This patch effectively defers the calculation of the width/height to the time when we reflow, instead of relying on the drag handler's calculations to be in sync with what we end up doing after reflow.  I also wrote a test which triggers this bug before the patch being applied.  And as a final bonus, the patch only removes code.  ;-)

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 507039 [details] [diff] [review]
Patch (v1)

Lovely

Comment 13

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/6e590b267675