When a child element is placed outside of it's parent element's bounding rectangle it does not register with event.target. Elements placed inside of the parent elements bounding rectangle work fine.

I think the events are going to the wrong element to begin with - the clicks on the out-of-border children seem to be going to the BODY node rather than the positioned element. This is yet another case of events going to the wrong target.

I can see "Child clicked" in console window on clicking the child element. May be it got fixed. Let reported verify it.

In 1999-09-29-08-M11 Linux apprunner, it doesn't seem fixed. If I click one of the pink boxes outside the blue box, I see: [object HTMLHtmlElement] If I click one of the pink boxes inside the blue box, I see: [object HTMLElement] child clicked These messages should be the same. There are a whole bunch of bugs (at least 5 or so) about events going to the wrong element. Some of them (e.g., bug 1413) have been around for a long time. I'm not even sure whether that's an event handling issue or whether the event system is just getting incorrect information from layout. (The bug on AREA elements could also be related...)

Moving targetting issues to M13

Moving M16.

Fixed. There is still a small bug in that the div hangs out of its border slightly but clicking on the squares works fine.

Don't know whether the regressed or was never fully fixed. But it doesn't work. The upper left square doesn't work right. I suspect it may be a dupe of the negative margines bug but reopening anyway for further investigation.

This problem has never been fixed!!! I filed this bug in responce to the work I'm doing on the image map editor for ender. I am usine the exact same kind of code for teh resize handles on the shapes. When I have a rectangle which has 8 handles (1 top left corner, 1 top center, 1 top right, 1 right center, 1 bottom right, 1 bottom center, 1 bottom left, 1 left center). The only ones that work properly are bottom center, bottom right, and right center, the other 5 do not. It seems to me that if a child object hanges either off the left or top edge of an element event.target is not registered properly. I will change the test case to reflect the exact situation I'm dealing with.

Another thing I noticed is that if a child element hangs partialy in and partialy out of the left or top of the parent, the portion inside the parent registers but the portion outside does not. I have also modified the test to reflect this.

I think the problem (remaining) here (after the changes I have in my tree are in) is a bug in the view system. Dump views on the above test case shows that the views for the larger blocks (absolutely positioned elements get their own views) have their bounds expanded to hold the child views to the right and bottom, but not to the top and left. The changes I have in my tree will thus fix this on the right and bottom, but not the top and left.

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

troy, beard - What should the views bounds be in this case? Should views completely contain their children, or should their origin coincide with the frame for which they were created? (Right now we seem to do the former on the right and bottom, and the latter on the top and left. See my previous comment.)

No views don't completely contain their children. The child views are allowed to stick outside their parent view. Then depending on whether the parent view is clipping children and the clip rect they may or may not be visible

This bug is preventing me from finishing my work on the image map editor! Please up it's priority so it will be fixed ASAP!

Is it possible that this bug and Bug 13215 are related? Should they be marked as dependant on one another?

Hmm. When I load this currently on Windows all of the non-responsive areas (anything to the left or top or the containing div) are initially not painted or possible painted over. Since the event handling is done in reverse paint order it seems likely to be tied in.

If there are painting problems then maybe bug 13215 is back. The problem with this bug is the way the view is sized. We need to decide whether the view code should change or the event handling code should change. The view code is internally inconistent too (it expands to hold children on the bottom and right). See my 03-19 comment.

M16 has been out for a while now, these bugs target milestones need to be updated.

Updating Milestone to M18.

kmcclusk - I'd like to discuss the view issues involved on this bug with you sometime. This bug isn't that hard to fix, but we need to decide what the right thing to do is.

Retitling and nominating for nsbeta3. We should really fix this.

Taking this bug to keep it on my radar, and since joki is on sabbatical.

correctness of event routing among elements (child elements).

Mass update: changing qacontact to ckritzer@netscape.com

My attempt to fix this by commenting out the PointIsInside check in nsView::HandleEvent caused major problems with comboboxes (e.g., at the top of this page). They started getting events that they shouldn't have...

Giving back to joki since I was too lazy to fix it while he was away.

Marking nsbeta3-. Possible workaround would be to restructure the page.

PDT: Renominating for nsbeta3+ Hmmnn...this sounds a little too serious to just ask the users to restructure their web pages (sorry Nisheeth). Clearing nsbeta3 resolution.

I just tried using GetExtents instead of GetBounds, and it caused weird results too...

We are really overloaded and this just doesn't make the cut. Sorry! Help wanted and appreciated!

This is really evil. The assumption that the coordinate system for a frame is the same as the coordinate system for its view is deeply entrenched.

I have a fix. It's a bit tricky but not as bad as I thought. The testcase works fine, trickier variations (e.g. widgets in the child views) seem to work fine too. The general idea is to extend nsView so that its bounds can extend above and to the left of its frame's origin (I call the delta the "frame offset"). The top-left corner of the bounds is not revealed to client code using Get/SetPosition; it is only revealed by a call to GetBounds(). I modified almost all of layout to use GetPosition()/GetDimensions() instead of GetBounds(), that job should actually be finished to get this completely right. nsViewManager is modified to be aware of the frame offset and use GetBounds where necessary. The tricky thing is that the origin of the view's coordinate system remains the origin of the view's frame, which is no longer the same as the top left corner of the view's bounds. Even so, remarkably few changes were required. If you think this might be checkin-able, I can try to backport the fix to nsViewManager2.

This is definitely a view bug --- reassigning. I'm going to attach a patch. This patch changes the view interfaces slightly; in particular it changes the meaning of nsIView::SetBounds and nsIView::GetBounds, because their currently intended meaning is quite unclear in the presence of child elements extending above or to the left of the view. The patch also adds a new method to nsIViewManager so that layout can tell the view manager about views that have sticking-out children. Naturally, the patch also makes all necessary updates to use these new interfaces correctly. The patch does NOT, however, fix this bug; it just makes it possible to fix the bug by changing only the view manager. In fact, the patch should not change observed behavior at all. The case for checking this in is simple: then I can produce another SMALL patch which adds nsViewManager to the build and provides a pref to switch view managers, so people can test this out easily.

Robert: Good work on tracking down the core problem here. I would prefer not to change the meaning of SetBounds and GetBounds. If we need to track the bounds of the view and all of its contents lets create a new bounds for that purpose. The original design was that the views should not enclose their children. By modifying the mBounds so views enclose their children we are changing this core design. I believe with your change, instead of having the frames modify their views so they are always large enough to enclose their children, the views are doing it automatically. I think we could fix this bug without modifying the meaning of mBounds by checking for views that are extend outside their parent view and make sure they are traversed inside nsView :: HandleEvent. If it is too inefficient to compute whether the view is outside the parent view each time during HandleEvent we could cache an mEventBounds for the view. (Which would be essentially the same thing your doing with mBounds in your patch.) But at any rate, I think it is desirable to keep the mBounds distinct from the EventBounds. The layout team and others will be impacted by this change so I'm CC'ing a bunch of layout people buster@netscape.com, attinasi@netscape.com, waterson@netscape.com, karnaze@netscape.com

The meaning of the view bounds was changed quite some time ago, and not by me, so that the view encloses all the child frames that extend beyond the main frame in the positive x and y directions. This code has been true in the trunk for many months. I am just trying to make things consistent so that it encloses children in the negative x and y directions as well. Before and after my change, the frames are responsible for sizing their views to contain all their frame children. This happens in nsContainerFrame::SyncFrameViewAfterReflow. This does not just affect events. It also affects painting. It also affects the sizing and positioning of the view's widget, if it has one. We could have a design where a view has two bounds: one set of bounds that is identical to its frame's bounds (in the coordinate system of the parent view, of course), and one set of bounds that encloses all children. I think we would find that the first set of bounds is not useful, except to get the frame's origin in the coordinates of the parent view. Such a design represents a deviation from existing assumptions, just as my design does.

What I'm meant by "the views should not enclose their children" was that the views shouldn't modify their bounds to enclose their child views. But it is perfectly acceptable for the frames to size and position the views to enclose all of the child frames. If they don't the frames will not be visible of course. If the frame's want to make sure they enclose the views with negative offsets that fine, but the view system should not expand their sizes automatically to make this happen. I may be mistaken, but I had the impression that was what your patch was doing (i.e expanding the parent view to enclose it's child views with negative offsets.)

No, each view is sized and positioned entirely by its frame, both before and after this patch.

I see this on Macintosh.

Adding rtm in hopes that someone can fix this! This is a big problem and will seriously stunt Netscape/Mozilla DHTML capability. I also believe it is directly related to Bug 13215. Please if you can do anything to fix this, do so ASAP! If a patch is in the works, or you have some other status on this bug please post it here. Dan

I have a patch in my tree which fixes this, but apparently Kevin's not sure it's the right fix, and is too busy to discuss it. So, we wait.

Kevin can you PLEASE look at Robert's patch??? If this doesn't get fixed soon my entire project could get dumped from the NS branch. Dan

Dan, I'm afraid I have to tell you that the chances this will be fixed in the NS6 branch are approximately zero. The changes required are not very numerous or particularly hard, but they are quite fundamental, and will almost certainly be rejected by the PDT. The best we can hope for is to get this fixed on the trunk.

So Robert, by "fundamental" are you saying that the changes would be too high-risk to attempt on the branch at this point?

My patch changes the semantics of some of the frequently-used nsIView methods, but in a way that does *not* change behavior in the common case where a view's frame has no child frames sticking out of it above or to the left. So I think it's low risk, but I suspect PDT will disagree. And of course there's the fact that last we heard, Kevin either didn't understand the patch or didn't think it was the right fix. If I'm wrong and PDT actually does want the fix for this, then I'd have to backport the view manager fixes from the new view manager to nsViewManager2. I'm not that interested in writing patches that will most likely be rejected by PDT and then immediately obsoleted when the new view manager lands. So I'd like to see PDT interest --- ideally, the view interface change patch checked in --- before I do that work.

Unfortunetly, this change is too large and too many people would have to OK it to get in for rtm. Marking rtm-. We need to discuss this immediately after RTM. Dan: perhaps we could try to come up with a work around which solves your problem. Please contact me through email.

I did come up with a workaround for my project (I moved the children inside the parent), but I have to say I am still very disappointed about this! I filed this bug over a year ago and it was ignored until the last minute when it was too late to fix! This is going to be a huge blow to Netscape's DHTML ability and I believe it's going to draw a lot of negative reviews from the DHTML community. I only wish I would have pushed this one harder earlier on, then maybe someone would have realized how big of a problem it was and fixed it when they still had a chance. :( All I can hope for now is that we can get this checked in to the trunk so that it will be fixed in the next point release. Dan

Robert: I have just gotten around to taking a another look at this patch. Sorry for the long delay. The patch appears to allow the origin for the view's bounds to be set. Rather than having the origin always being the upper left hand corner, the origin for the bounds is set using the frame offset. Is this a correct interpretation? If so, could we change the terminalogy from frameOffset to boundOrigin or viewOrigin. In general the view system is independent from frame system. Frames know about view's but view's don't know about frames, so it would good to use more general terminology within the view system to express the bounds offset. What do you think?

Setting the view's origin should also be useful for implementing the css outline property.

CSS "outline" is just another example of where a frame's content overflows its bounds. I think that layout already computes the overflowing rectangle correctly taking "outline" into account; once we've fixed this bug here, we should be 90% of the way to correct "outline" support. I agree that the terminology is confusing and we need to fix that. Your point about not mentioning frames is a good one! Each view has two important properties: -- a rectangle which encloses the content of the view and all its subviews -- a point which establishes the origin of the view's coordinate system Currently the first property is called the "bounds" and is given in (x,y,w,h) coordinates relative to the parent view. The second property is forced to be the top left corner of the bounds. I propose we call the first property the "view bounds rectangle" and the second property the "view origin". In the interface, we can specify both as being relative to the origin of the parent view. Normally the view origin will be equal to the top left corner of the view bounds rectangle, but obviously that won't always be the case. Do you think that's too confusing?

"-- a rectangle which encloses the content of the view and all its subviews". I would describe the bounds as the rectangular area which the view system manages for painting and event handling. It shouldn't be described as enclosing it's subviews. If the frame wants to expand the size of the view and reset the view's origin to make this happen that's fine, but the view system doesn't enforce this or require it.

I propose we call the first property the "view bounds rectangle" and the second property the "view origin". The view origin is a little "odd" in that we really are specifying the bounds origin. If something were labelled "view origin", I would expect a child views position to be relative to the parent view's transformed origin (Comming from a 2-D graphics/tranformation mind set), but the child view's position is relative to only it's parent view's position. The parent view's origin has no impact on the child view's position at all. Maybe we should just forget about calling it the origin altogether (to avoid confusion) and call it bounds offset?

> I would describe the bounds as the rectangular area which the view system > manages for painting and event handling. It shouldn't be described as > enclosing it's subviews. If the frame wants to expand the size of the view and > reset the view's origin to make this happen that's fine, but the view system > doesn't enforce this or require it. No --- nsViewManager and nsViewManager2 both require it. For example, if a view's bounds extends beyond the bounds of its parent view, then the child view doesn't receive mouse events properly. This is why this bug was reported in the first place! Requiring that a view's bounds enclose the bounds of its child views is a good thing. Without this invariant, every repaint or mouse event would require traversal of the entire view tree. > The view origin is a little "odd" in that we really are specifying the bounds > origin. If something were labelled "view origin", I would expect a child > views position to be relative to the parent view's transformed origin (Comming > from a 2-D graphics/tranformation mind set), but the child view's position is > relative to only it's parent view's position. The parent view's origin has no > impact on the child view's position at all. I'm confused and maybe you are too. What do you mean by "position"? The "view origin" I'm talking about is not the same as the "frame offset" used in the patch. I'm suggesting defining the "view origin" to be the point which is the (0,0) of the coordinate system defined by the view, and specifying it in coordinates relative to the parent view's "view origin". So indeed the parent view's origin does determine where the child view's origin is. Perhaps an example would help. In the example of the testcase, the view tree would look something like this: Root view: bounds=(0,0,1024,768), origin=(0,0) Child view: bounds=(430,30,240,240), origin=(450,50) Child view: bounds=(-20,-20,20,20), origin=(-20,-20) Child view: bounds=(-20,200,20,20), origin=(-20,200) Child view: bounds=(200,200,20,20), origin=(200,200) Child view: bounds=(200,-20,20,20), origin=(200,-20) ...

"No --- nsViewManager and nsViewManager2 both require it. For example, if a view's bounds extends beyond the bounds of its parent view, then the child view doesn't receive mouse events properly. This is why this bug was reported in the first place! " How do we handle the case where the child view's are clipped by their parent view? see troy's comment: "No views don't completely contain their children. The child views are allowed to stick outside their parent view. Then depending on whether the parent view is clipping children and the clip rect they may or may not be visible." Would we expand the parent view's size to contain it's children only if overflow:hidden has not been specified? I agree that the view system can not handle events and repainting of views which extend outside the parent view's boundaries without adding a bunch of code, but I was assumming we would leave it up to the frame to correctly size the view by looking at the overflow property. "I'm suggesting defining the "view origin" to be the point which is the (0,0) of the coordinate system defined by the view" Ok. I understand. I was thinking about it from the perspective of how do we set the view's origin relative to it's coordinate space (bounding box). With your definition setting the view origin and the current method SetPosition are equivalent. We still need a method for setting the bounding boxes upper left hand corner relative to the origin. I was thinking we would just call it SetBoundingBoxOffset.

> see troy's comment: Good point. OK, so the invariant is this: IF a view is not clipping its child views, then its bounds must contain the bounds of its child views. > I was assumming we would leave it up to the frame to correctly size the view > by looking at the overflow property. Yes! The invariant is maintained by the frame code, not the view system, and we should keep it this way. We may wish to add assertions to the view API to check that the frame code is doing the right thing. > With your definition setting the view origin and the current method > SetPosition are equivalent. We still need a method for setting the bounding > boxes upper left hand corner relative to the origin. I was thinking we would > just call it SetBoundingBoxOffset. Actually, SetBounds (plus a little arithmetic) can be used to do this. SetFrameOffset is not needed. I'll remove it. Assuming I do this, add comments summarising this discussion, and remove all references to frames, are we converging on something you're happy with? I think eventually we should take all the "update" methods out of nsIView and force the updates to go through nsIViewManager --- almost all modifications to the view system already go through nsIViewManager anyway. It would be neat to make nsIView a small read-only interface. The view system itself could use a private interface to the views to do updates. (Ultimately we could get rid of the nsIViewManager* pointer from nsView and pass it as a parameter everywhere it's needed.)

"Assuming I do this, add comments summarising this discussion, and remove all references to frames, are we converging on something you're happy with?" Yes, I think were there. From the view modules perspective we have simply removed the limitation that the origin of the view is also the upper left hand corner of it's bounding box. We'll also have to post a message to the layout news group to make sure frame authors are aware of this change. Thanks for your patience in getting this hashed out. :-) "I think eventually we should take all the "update" methods out of nsIView and force the updates to go through nsIViewManager" I agree.

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

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

Milestone 0.8 has been released. We should either resolve this bug or update its milestone.

Setting target to future.

Nom. nsbeta1.

Correct QA Contact->petersen

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

roc, kmcclusk: So, what's the status of this bug? It really is a rather serious bug, and it would be nice to see it fixed.

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

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

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

I'm working on the patch for this. It doesn't quite work yet, but it's pretty close. It's going to be a little bit risky because there are a lot of places where we assume view (top, left) == view's frame (top, left) and view's frame (top, left) == view's widget (top, left), and many of those assumptions must be broken.

OK, here it is. It's not very tricky, but there are a lot of places that needed to be changed. I have checked all the testcase in dups of this bug and they all seem fine (except for one or two which need general display-list-aware event handling, which is another project). Risk: I touched a lot of code, but most of the changes should have no effect in the common case where frame (top, left) and view (top, left) are the same. Performance: Should be miniscule but given the size of the patch, maybe we should run some page load tests?

Note that with this patch in hand, it becomes very easy for frames to reliably paint outside their bounds if they need to; just set the combinedArea during reflow to whatever you need. This should make stuff like CSS2 'outline' and 'text-shadow' much easier to deal with, maybe other stuff too. One issue which came up while I was doing this: what should we do about 'overflow: scroll' containers which have content above and to the left of their origin? Presumably we should be able to scroll to view all of it. Presumably the initial scroll position should be at the container's frame origin. Definitely this would be a lot more work. Currently we can only scroll right and down from the frame origin.

jgrm: Could you apply Robert's patch (id=64964) and run your page-loader tests on one platform when you get a chance? Thanks!

Given that this seems to fix bug 120934, do we want to try to push this for mozilla0.9.8, or are we not confident in this fix yet?

I'm not confident. In fact I think there is a bug in popup positioning with this patch. I will try to extract the relevent parts for bug 120934.

I tested (win2k) the current trunk vs. the current trunk with attachment 64964 [details] [diff] [review] (backing out rev 3.222 to nsViewManager.cpp to avoid a conflict). I did not see a significant difference (greater than 1%) in the times for the page load test. (Sorry for not getting to this earlier).

Great, thanks!

kmcclusk, waterson --- I'd like to get this reviewed and checked in ASAP. It's risky so we should do it as early on in the milestone as possible.

Except I want to track down one bug first. So don't rush, just a heads-up :-).

OK. I thought there was a popup bug but I can't reproduce it. I might have been looking at the wrong build. So, please review the attached patch. Thanks!

Comment on attachment 64964 [details] [diff] [review] THE FIX Hope it works. rs=waterson.

You sure it's not that it causes you to accidentally resolve bugs as WORKSFORME? :-)

Good work Robert! I don't see any problems with the patch. I think it could use a little more explanation of how the view coordinate system works with this change. Something along the lines of: SetPosition in conjuction with SetDimension establish the local coordinate space for a view. SetPosition establishes the view's origin (0,0) relative to it's parent's origin. SetDimension establishes The view's horizontal and vertical extent relative to the view's origin. For example: to determine the absolute coordinate for a view's upper left hand corner (It's offset from the root view) you would compute the sum of all of the view's ancestor's positions (mPosX, mPosY) then add the result to the view's dimension offset (mDimBounds.x, mDimBounds.y) The following comment is a little confusing: nsView.h nsRect mDimBounds; // relative to parent Isn't the mDimBounds rectangle relative to the view's mPosX, mPosy and the view's mPosX, mPosY is relative to it's parent's mPosX, mPosY.

Comment on attachment 64964 [details] [diff] [review] THE FIX r=kmcclusk@netscape.com

No, actually that comment is correct. mDimBounds is stored in coordinates relative to the parent view. Thus in the normal case, mDimBounds.x == mPosX and mDimBounds.y == mPosY. That is why nsView::GetDimensions subtracts out the mPosX and mPosY and nsView::GetBounds doesn't. dbaron: OK, testing out the bugzilla comboboxes before submitting a comment was a bad idea :-). Chris: thanks for the vote of confidence :-). After checking this in I will post to .layout and mention the API change this entails: namely, you have to use nsIView::GetPosition when you want to translate coordinates from one view coordinate system to another, but you have to use nsIView::GetBounds if you want to find out the area covered by a view.

Checked in.

Marking FIXED