377391 - Parts of a rectangle with CSS outline remains, when changing rectangle size

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Daniel Kirsch]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
Build Identifier: 

A graphical bug occurs when drawing a div used as a selection rectangle. I've sized a dashed rectangle large and back to a smaller size.
The border is set by: -moz-outline:1px dashed #888;

It works when using "border" instead of "-moz-outline".

Reproducible: Always

Comment 1

[Daniel Kirsch]

Attachment: Screenshot showing the effect

Comment 2

[Daniel Kirsch]

Tested with current nightly:
Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.9a4pre) Gecko/20070412

Comment 3

[Daniel Kirsch]

Attachment: Testcase showing the effect

The testcase should show the effect.

Comment 4

[Ria Klaassen (not reading all bugmail)]

Regressed on 26 Feb 2007.

Comment 5

[Gérard Talbot]

The residual trailing outlines do not appear when resizing from bottom to top
and from right to left (at least at the left hand side of the cursor point of
origin).

Comment 6

[Eli Friedman]

Attachment: Patch

I'm not completely sure this is right, but what we were doing before is definitely wrong.

Comment 7

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

Why do you think this is wrong? The idea is to draw the CSS outline around the overflow returned by this frame's Reflow (which includes the union of the overflow of all children)

Comment 8

[Eli Friedman]

Attachment: Patch v2

Oh, oops; the previous patch was wrong.  I think this is correct... invalidation is really a pain.

Comment 9

[Eli Friedman]

(In reply to comment #7)
> Why do you think this is wrong?

I'm pretty sure it's wrong to be using the new overflow rect.  I can't seem to come up with a testcase, though.

Comment 10

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

It's really a design decision. CSS2.1 doesn't say exactly where it should be drawn.

Comment 11

[Eli Friedman]

(In reply to comment #10)
> It's really a design decision. CSS2.1 doesn't say exactly where it should be
> drawn.

I'm pretty sure it's wrong to be using the *new* overflow rect. (As opposed to the old one.)

Comment 12

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

The relevant comment is this (from nsIFrame.h):

 For an incremental reflow you are responsible for invalidating
 any area within your frame that needs repainting (including
 borders). If your new desired size is different than your current
 size, then your parent frame is responsible for making sure that
 the difference between the two rects is repainted.

So strictly speaking, the current nsAbsoluteContainingBlock code is correct. But this implies that if a frame's overflow area or size changes, then the frame is responsible for repainting the difference between its old and new overflow area, minus the difference between its old and new size, which is stupid.

I'm tempted to change the contract here to something more sane. Alternatively we can keep this contract, and have FinishAndStoreOverflow invalidate "(oldOverflowRect union newOverflowRect) minus newSizeRect" always. What do you think?

Comment 13

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

Hmm, actually, what I think we should do is close to your first patch. CheckInvalidateSizeChange, in the anyOutline case, should invalidate r *and* the old overflow area.

Comment 14

[Eli Friedman]

I was thinking about that, but then I noted that the current code in nsBlockFrame::ReflowLine works
the same way as my proposed change, as far as I can tell.  We should probably make it consistent, whatever we decide.

Comment 15

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

I think we should simplify ReflowLine so that we only test oldBounds.TopLeft() != newBounds.TopLeft() to see if the line moved. If the line didn't move then we should just reflow the difference in frame rects and rely on CheckInvalidateSizeChange to save us in other cases. We'd better check the test(s) in bug 244017 though.

Comment 16

[Daniel Kirsch]

Attachment: Screenshot for 12px border object

This doesn't seem to affect only outline. A similar effect happens for non solid borders when changed. However, as far as I can tell only for the part where a solid border would be.

The screenshots shows a div with border: 12px dotted #f00
This also happens for dashed and double border.

Is this a new bug or related?

Comment 17

[Eli Friedman]

Attachment: Patch v3

Now invalidating both the old and new overflow rects.

Comment 18

[Eli Friedman]

Checked in.