Closed Bug 9809 Opened 22 years ago Closed 17 years ago

[outline]Dynamic changes of outline properties aren't repainted

Categories

(Core :: CSS Parsing and Computation, defect, P4)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: alla, Assigned: roc)

References

Details

(Keywords: css2, testcase)

Attachments

(1 file, 2 obsolete files)

When the outline property of some content changes (eg. it tracks focus with
outline) the difference in the styles are not noticed byt the style system,
leading to no repaint.

The cause of this is that StyleSpacingImpl::CalcDifference doesn't compare
outline data.
Attached patch Fixes the problem (obsolete) — Splinter Review
Reassigning peterl's bugs to myself.
Accepting peterl's bugs that have a Target Milestone
Pushing my M15 bugs to M16
Keywords: css2
Is this still a problem?  I know there are still problems with outline
repainting, but are they with the *region* repainted?
Keywords: patch
Summary: [Patch] Dynamic changes of outline properties aren't repainted → Dynamic changes of outline properties aren't repainted
The style code was fixed a while ago to generate a repaint when the outline 
changes (StyleSpacingImpl::CalcDifference() was returning NS_STYLE_HINT_VISUAL). 
Problem: a repaint wasn't sufficient to dynamically update the display. See the 
the end of the "kipp@netscape.com  1999-07-15 13:26" comment in bug 9816 for more 
info. In order to work around some aspects of the problem, I changed the code to 
return NS_STYLE_HINT_REFLOW.

We are now left with 2 problems which are both related to the outline area that 
Kipp and Troy were talking about in bug 9816.
- We shouldn't need to return NS_STYLE_HINT_REFLOW when the outline changes.
- The outline doesn't paint over surrounding frames. See the testcase that I'm 
going to attach.

Reassigned to Troy to add the outline area in nsIFrame.
Assignee: pierre → troy
Status: ASSIGNED → NEW
Attached file testcase
Blocks: 24676
Rod, I think you're the one who's doing outlines so re-assigning to you
Assignee: troy → rods
Depends on: 30671
Blocks: 28522
changed summary
Status: NEW → ASSIGNED
Summary: Dynamic changes of outline properties aren't repainted → [outline]Dynamic changes of outline properties aren't repainted
*** Bug 18579 has been marked as a duplicate of this bug. ***
*** Bug 30671 has been marked as a duplicate of this bug. ***
*** Bug 32893 has been marked as a duplicate of this bug. ***
*** Bug 34799 has been marked as a duplicate of this bug. ***
I've been thinking about a workaround to fix this (unless someone wants to write
an invalidation API like kipp mentioned in bug 9816), and, well I want to write
down my thoughts here to see what others think about it and so I don't forget
them (and so I can close all these LXR windows I have open).

Anyway, it might work to:

* add an additional style change hint called NS_STYLE_HINT_DOUBLE_VISUAL between
NS_STYLE_HINT_VISUAL and NS_STYLE_HINT_REFLOW

* change nsStyleSpacing::CalcDifference at
http://lxr.mozilla.org/seamonkey/source/layout/base/src/nsStyleContext.cpp#790
to return the new NS_STYLE_HINT_DOUBLE_VISUAL (meaning the frame would have to
be invalidated with both old and new data)

* Change
http://lxr.mozilla.org/seamonkey/source/layout/base/public/nsStyleChangeList.h &
http://lxr.mozilla.org/seamonkey/source/layout/base/src/nsStyleChangeList.cpp so
that nsStyleChangeData also has an nsCOMPtr<nsIStyleContext> previousContext (or
maybe make it a raw pointer and/or use it only for outline hints???)

* change CaptureChange at
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsFrameManager.cpp#1136
to store the old style context using the above change

* Change ProcessRestyledFrames at
http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsCSSFrameConstructor.cpp#9099
to call something that does the proper invalidation (this is the part I haven't
thought through completely yet).

That still leaves issues with views and the problems that Pierre's attachment
demonstrates... but does this seem like a sensible approach for propogating the
style information?  (The problem I'm talking about fixing still exists,
right...?)
moving to M17
Target Milestone: M16 → M17
Outlines paint on the outside of a frame and curently there is no mechanism 
built into frame to make the view large enough to contain the frame plus outline
Assignee: rods → karnaze
Status: ASSIGNED → NEW
Marking remind. This will not make release 1.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → REMIND
Target Milestone: M17 → M20
*** Bug 38773 has been marked as a duplicate of this bug. ***
*** Bug 30122 has been marked as a duplicate of this bug. ***
We cannot ship FCS with this still buggy -- this makes the 'outline' property
unusable in many cases. We really should either remove support for 'outline' 
altogether or fix this.

Reopening.
Status: RESOLVED → REOPENED
Resolution: REMIND → ---
See my notes in bug 24676: it looks like outlines have been disabled already.
Kevin, can you or Rod do the workaround that was discussed. There is not enough 
time for the more elaborate solution. 
Assignee: karnaze → kmcclusk
Status: REOPENED → NEW
Rod, what's the status on supporting outline?
Assignee: kmcclusk → rods
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another 
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration. 
Status: NEW → ASSIGNED
Target Milestone: M20 → Future
*** Bug 45498 has been marked as a duplicate of this bug. ***
bug 18579 alone had 7 dups before marked dup of this
Adding mostfreq keyword.
Keywords: mostfreq
As per meeting with ChrisD yesterday, taking QA.
QA Contact: chrisd → py8ieh=bugzilla
blah. Anything I can do to help?
on linux the outline of dropdown boxes don't vanish before you click in page.
Many bugs about that phenomena have been marked dup of this bug.
Many of them - identical - have also instead been marked dup of bug 39992
39992 has 11 dups already. Is it a dup of this one or not?
Bug 39992 has nothing to do with the 'outline' property.
jce2, we would love to help on this issue except it is quite involved which is 
why we are postponing it until 6.x.

The issue is that outlines need to painted on the outside of a frame. Frames are 
contained inside Views. In some cases the view may have to be enlarged to hold 
the frame plus the outline (like absolutely positioned elements). Then there are 
all the issues with invalidating/clipping/painting just the outline which may 
actually (or most likely) be part of the frame's parent frame. Frames know about 
views, but views don't know about frames. So this all seemed like a bit too much 
to bite off for 6.0.

But we are very open to any research or investigation in this area. And if you 
were to find that the impact turned out to be small and of lower risk we could 
always consider it for 6.0, or at the very least it would give us a great jump 
start on the issue for 6.x  Thank you for offeeing your help, let us know if you 
are still interested in looking into this issue.
David Baron: if 39992 has nothing to do with this bug, then many of the dups
here aren't dups at all.
You're right.  All the dups of this bug since May (I think -- haven't checked
quite all yet) are really dups of bug 39992.  This is a totally different bug. 
Those bugs should not have been verified duplicates of this bug.  People
verifying dups should be more careful.
When bug 13213 is fixed, we should be able to take a big bite out of this bug by 
ensuring that HTMLReflowMetrics::mOverflowArea contains the outline area (if 
any). Then the view will automatically be sized and positioned to include the 
outline area.
Depends on: 13213
marking invalid, because we currently aren't implmenting this per the spec.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago21 years ago
Resolution: --- → INVALID
Reopening. Once we switch '-moz-outline' to 'outline' then we'll have to fix 
this bug.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Reassigning to Robert to get this off Rod's open bug list. Robert: I hope that's
ok. If it's not then feel free to reassign it to me and I'll keep it; however
I'm likely to forget about it if it is assigned to me as I don't look at my
assigned bugs list very regularly! :-)
Assignee: rods → roc+moz
Status: REOPENED → NEW
Keywords: mostfreq
Status: NEW → ASSIGNED
Priority: P3 → P4
Blocks: css2outline
Attachment #872 - Attachment is obsolete: true
Keywords: patchtestcase
The view manager work that landed last milestone has made this bug solvable, but
it's still not 1.0 material.
Hardware: PC → All
Target Milestone: Future → mozilla1.1
*** Bug 148684 has been marked as a duplicate of this bug. ***
Blocks: 150098
*** Bug 150098 has been marked as a duplicate of this bug. ***
*** Bug 152858 has been marked as a duplicate of this bug. ***
*** Bug 161625 has been marked as a duplicate of this bug. ***
*** Bug 169169 has been marked as a duplicate of this bug. ***
We're getting close to being able to fix this. Soon I should land code that
computes a correct overflowArea for outlined elements, so views for outlined
elements will be sized correctly.

There's still the problem of what to repaint when outline changes. dbaron
described a way that involves looking at the old style data. I think an easier
way would be to add a style hint that forces invalidation of a frame's entire
view, nsChangeHint_RepaintView.

There are other issues in the duped bugs, like outline painting not respecting
clipping. We'll have to look at those duped bugs before we can decide we have
working outline.
There's also the issue of outlines on elements that don't have views.

See also the discussion in bug 151375.
Blocks: 151375
RepaintView would repaint the nearest containing view containing the frame, so
taking care of frames without views as well.

There certainly are various other things that would need to be fixed. For
example during reflow we'd have to invalidate the outline area whenever we
invalidate the border area. And other things I haven't thought of...
We'll have to modify event handling so that mouse events in the outline area go
to the outlined element. I think.
A better approach would be to simply ensure that a frame with 'outline' always
has a view.
You want to create and destroy views every time the focus changes?
We only need to create a view for a frame when it first becomes focused. There's
no harm in leaving a view attached to a frame that doesn't really need it
anymore, so there's no need to destroy views when the outline is removed.

If even that is considered too expensive, we can improve the way we create
views. Right now we do a FRAMECHANGE if a view might be needed after a style
change. As you know plans are well advanced to add a FrameNeedsView hint which
will do a FRAMECHANGE only if there isn't already a view. We can even improve
that by instead of upgrading FrameNeedsView to a FRAMECHANGE, just creating a
new view for the existing frame, inserting it into the view hierarchy, and
reparenting the views for the descendant frames of the restyled frame to point
to the restyled frame's new view. That would be followed by a reflow of the
restyled frame to give the new view the correct geometry.
That would be a lot faster than a FRAMECHANGE and also bypass the brokenness of
the event state manager w.r.t. FRAMECHANGEs.
One other thing: the style hint for 'outline-width' will end up looking like
nsChangeHint_FrameNeedsView | nsChangeHint_RepaintView | nsChangeHint_ReflowFrame

We need to reflow when the width changes so that the overflow area(s) get
updated correctly.
No longer blocks: 150098
No longer depends on: 30671
*** Bug 159290 has been marked as a duplicate of this bug. ***
*** Bug 202761 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla1.1alpha → ---
The testcase no longer works. It doesn't seem like we support css outline at all.
Using -moz-outline the test case works for me.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago17 years ago
Resolution: --- → WORKSFORME
The point is that outline (outside the border) is harder than -moz-outline
(inside the border) and this bug needs to be fixed before we really support outline.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Attachment #151835 - Attachment is obsolete: true
Are we still going to support -moz-outline, or are we just going to make it work
outside the frame and rename it outline then.

I thought it was just temporarily renamed to -moz-outline into it worked correctly.

Fixed by checkin to bug 151375.
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Hey Guys! Good Job!
You need to log in before you can comment on or make changes to this bug.