241314 - Can't mouse though images (nsStackFrame::GetFrameForPoint not implemented)

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[[e-mail]]

Steps to reproduce problem:
1. Load test case
2. Try to hover over the upper sets of squares
3. Try to hover over the lower sets of squares

The difference between the two examples is the use of box instead of image.
I used EditorFormatToolbar.css as a convenient source of hover styles.

Comment 1

[[e-mail]]

Attachment: Test case

Comment 2

[Boris Zbarsky [:bzbarsky]]

I'm a little confused here.  What is the expected behavior?  Why?  A _simple_
testcase would have helped.

Comment 3

[[e-mail]]

Well the expected behaviour is that the hover effect works on both sets of
boxes. Also there are a number of ways of achieving an image underlay/overlay
and I wanted to demonstrate that all of them block hover.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So is the image below the box that should be getting hover in every single one
of those testcases?  Note that width="0" height="0" is ignored since stacks
stretch all kids out to the size of the stack.  If no order is specified does a
stack stack its kids in document order top to bottom or bottom to top?

Comment 5

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

nsStackFrame needs to override GetFrameForPoint in a useful way.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, it doesn't?  I was going to look for bugs in its override after figuring out
what the behavior _should_ be, but I guess that's sorta moot... ;)

Comment 7

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

GetFrameForPoint should do whatever paint does. :-)

Comment 8

[Boris Zbarsky [:bzbarsky]]

Yeah.. paint...

nsStackFrame does not implement Paint(), so it inherits it from nsBoxFrame.

It _does_ override PaintChildren -- to only call through to
nsBoxFrame::PaintChildren if the layer is NS_FRAME_PAINT_LAYER_BACKGROUND.

Then in PaintChild() (which it also overrides) it totally ignores aWhichLayer
and paints the background, then the foreground for the child.

So logically, the thing to do for GetFrameForPoint would be to return nothing in
the foreground and walk the kids in reverse order for background, asking for
points in both layers (sorta what nsStackFrame::GetStackedFrameForPoint tries to
do... except no one calls it).

Comment 9

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

Attachment: patch

This splits out a separate child method from nsBoxFrame::GetFrameForPoint and
overrides both in nsStackFrame (although the outer only for performance).

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146794 [details] [diff] [review]
patch

>Index: nsBoxFrame.cpp
>+nsBoxFrame::GetFrameForPointChild(nsIPresContext*   aPresContext,

>+      // if the kid had a child before see if this child has mouse
>+      // though. 

I realize you just copied this, but what it should probably say instead is:

"If we had a lower frame for this point, check whether hit's box has mouse
through.  If so, stick with the lower frame that we found"

or something along those lines...

r+sr=bzbarsky with that.

Comment 11

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

(In reply to comment #10)
> I realize you just copied this, but what it should probably say instead is:

moved, not copied :-)

Comment 12

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

Fix checked in to trunk, 2004-04-22 11:59 -0700.

Comment 13

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

Comment on attachment 146794 [details] [diff] [review]
patch

There's been a request that this go on the 1.7 branch, and it should be
reasonably safe.  There is a chance of regressions, but they seem like the kind
of things that would be noticed reasonably quickly, and it also seems more
likely to fix something obscure than to break it.

Comment 14

[chris hofmann]

Comment on attachment 146794 [details] [diff] [review]
patch

a=chofmann for 1.7

Comment 15

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

Fix checked in to MOZILLA_1_7_BRANCH, 2004-04-26 16:17 -0700.