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

RESOLVED FIXED

Status

()

Core
Layout: View Rendering
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Daniel Kirsch, Assigned: Eli Friedman)

Tracking

({regression, testcase})

unspecified
x86
Windows XP
regression, testcase
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
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
(Reporter)

Comment 1

11 years ago
Created attachment 261457 [details]
Screenshot showing the effect
(Reporter)

Comment 2

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

Comment 3

11 years ago
Created attachment 261459 [details]
Testcase showing the effect

The testcase should show the effect.
(Reporter)

Updated

11 years ago
Keywords: testcase
Regressed on 26 Feb 2007.
Blocks: 371536
Keywords: regression

Comment 5

11 years ago
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).
Assignee: general → roc
Component: GFX → Layout: View Rendering
(Assignee)

Comment 6

11 years ago
Created attachment 262200 [details] [diff] [review]
Patch

I'm not completely sure this is right, but what we were doing before is definitely wrong.
Assignee: roc → sharparrow1
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #262200 - Flags: review?(roc)
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)
(Assignee)

Comment 8

11 years ago
Created attachment 262203 [details] [diff] [review]
Patch v2

Oh, oops; the previous patch was wrong.  I think this is correct... invalidation is really a pain.
Attachment #262200 - Attachment is obsolete: true
Attachment #262203 - Flags: review?(roc)
Attachment #262200 - Flags: review?(roc)
(Assignee)

Comment 9

11 years ago
(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.
It's really a design decision. CSS2.1 doesn't say exactly where it should be drawn.
(Assignee)

Comment 11

11 years ago
(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.)
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?
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.
(Assignee)

Comment 14

11 years ago
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.
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.
(Reporter)

Comment 16

11 years ago
Created attachment 262275 [details]
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?
(Assignee)

Comment 17

11 years ago
Created attachment 265487 [details] [diff] [review]
Patch v3

Now invalidating both the old and new overflow rects.
Attachment #262203 - Attachment is obsolete: true
Attachment #265487 - Flags: review?(roc)
Attachment #262203 - Flags: review?(roc)
Attachment #265487 - Flags: superreview+
Attachment #265487 - Flags: review?(roc)
Attachment #265487 - Flags: review+
(Assignee)

Comment 18

11 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.