234847 - "overflow:hidden" doesn't hide events

Status: RESOLVED FIXED

(Core :: Layout: Tables, defect)

Description

[Jing Lim]

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6) Gecko/20040113

Try the following. Click on the RHS of the text, you still get an alert.

<table style="table-layout:fixed;width:50px">
<tr>
<td style="overflow:hidden;white-space:nowrap" onclick="alert('hi')">
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0
</table>


Reproducible: Always
Steps to Reproduce:
1. Display the html shown above.
2. Click on the right side of the '0 1 2 ..' text.
3.

Actual Results:  
An alert appears.

Expected Results:  
No event should occur outside of the clipped area.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: The testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Sounds like a GetFrameForPoint method somewhere is broken.... I wonder why...

This doesn't happen with blocks.

Comment 3

[Bernd]

=-=	Topic for #developers was set by
sipaq!~[e-mail] on Wed Feb 18 2004 23:58:25
GMT+0100
	bernd	bz: I just got bugspam about "overflow:hidden" doesn't hide events this
because overflow:hidden is a blatant hack
	bz	oh?
	bz	how so?
	bz	and why's it not for blocks? ;)
	bernd	don't you remmeber my clash with david when he broke my hack
	bz	doesn't.....
	-->|	Jesse (~[e-mail]) has joined #developers
	bernd	bz: you did sr it :-)
	bernd
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsTableCellFrame.cpp&branch=&root=/cvsroot&subdir=mozilla/layout/html/table/src&command=DIFF_FRAMESET&rev1=3.315&rev2=3.316
	botbot	SeaMonkey: 'Linux luna Depend' has changed state from Horked to Success.
	bernd	http://bugzilla.mozilla.org/show_bug.cgi?id=221140
	bernd	to make the story short only row group frames encapsulate their children
in a scrollable frame wehn overflow is specified
	bz	bernd: ah, ok. And tables just clip the painting?
	bz	bernd: or something?
	bz	er, table cells
	bernd	exactly
	bz	note that we _could fix this in an evil way
	bz	If overflow is hidden, unset the outside children flag before calling
GetFrameForPoint
	bz	then reset it if needed
	bz	er, before calling GetFrameForPointUsing
	bernd	bz: a hack for a hack sounds like a "good" plan to get a r- from david

Comment 4

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

You could just check the overflow value in GetFrameForPoint and return if it's
hidden and the point isn't in the rect.

Comment 5

[Bernd]

Attachment: patch

Comment 6

[Bernd]

I can add the hack comment from
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableCellFrame.cpp#471.

The issue here is that cell/row/table frames should create a scrollframe if the
overflow:hidden is specified. But they dont, so they will currently report a
overflow area. I think the patch is correct as a scrollframe will not return
overflow in this case.

Comment 7

[Bernd]

Attachment: patch rev2

Comment 8

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

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

Comment 9

[Boris Zbarsky [:bzbarsky]]

Bernd, is that patch set for review?

Comment 10

[Bernd]

yeah I did not run the rtest so I forgot that thing, but now I did run them, and
they remove the nonnecessary scrollbars in the overflow testcases like
http://lxr.mozilla.org/seamonkey/source/layout/html/tests/table/marvin/table_overflow_hidden_table.html

Comment 11

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

Comment on attachment 141796 [details] [diff] [review]
patch rev2

Please add a comment that the check for SCROLLBARS_NONE is needed only for
table elements, which implement it differently, and r+sr=dbaron.

Comment 12

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

But please test that programmatic scrolling of 'overflow: hidden' elements still
works correctly.

Comment 13

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

An example of what I mean in comment 12 is attachment 25609 [details].  And perhaps also
try modifying that by (1) putting something with a view inside it and (2) by
making the 'overflow: hidden' element 'position: absolute' as well.

Also, please fix the indentation (indent an additional two spaces).

Comment 14

[Bernd]

The testcase is really nifty, but ConsiderChildOverflow is used currently only
by table table frames and the sibling in mind, the fieldset frame. (
http://lxr.mozilla.org/seamonkey/search?string=ConsiderChildOverflow ). So I
guess I need first to modify either the testcase or the codebase to get this
code  tested in the way you proposed.

Comment 15

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

OK, then never mind the tests, but r+sr=dbaron with the other things addressed
(comment, indentation).

Comment 16

[Bernd]

Attachment: revised patch incl. david's comments

Comment 17

[Bernd]

I checked in the patch but referred to the wrong bug 233438