216977 - 'overflow:hidden' not working correctly on Linux (fine on Windows, DHTML Lemmings)

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P3)

Description

[Huw Lynes]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1

DHTML lemmings causes the following error in the javascript console.

Error: uncaught exception: [Exception... "Component returned failure code:
0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]"  nsresult:
"0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame ::
chrome://communicator/content/profile/profileSelection.js :: StartUp :: line 97"
 data: no]

This is coupled with a display error (multiple lemmings) see
http://users.pandora.be/wyffels.f/schermafdruklemmings.jpg for an example.

Reproducible: Always

Steps to Reproduce:
1.load the web page in mozilla1.4 or firebird0.6 on linux
2.navigate to the first level
3.open the javascript console to see the error

Actual Results:  
multiple lemmings resulting in an unplayable game. The developers believe this
to be a problem with the browser rather than the javascript.

Comment 1

[crisp]

I need to cut in here as the maker of the script.
I don't think the JS error has to do with this problem, nor that it is a JS 
error at all, but more likely a CSS/HTML render problem.

What happens is the following: I create a div with fixed dimensions and 
overflow:hidden. In that div I create an image, which has a larger width than 
the containing div. Apperantly the overflow:hidden is ignored and the extended 
part of the image is visible beyond the div.
In windows versions of Mozilla 1.3x and 1.4x this problem is not reproducable, 
so my guess is it is restricted to linux version but I'm not in a position to 
verify this myself at the moment.

I do have a much simpler testcase though: 
http://crew.tweakers.net/lemmings/desktop.html

notice how I create the elements and attach the css classes div.lemming and 
img.lemming

Comment 2

[crisp]

correction of a.m. url:

http://crew.tweakers.net/crisp/lemmings/desktop.html

Comment 3

[dolphinling]

Same display error on 1.5a and the latest nightly on Linux. 

In the JavaScript Console all I get is a bunch of "Error:  selected_level is not
defined" - but I've never used that before so I might be doing something wrong
there.

On the other hand, the game isn't unplayable, I got to the 3rd level and then
quit :-P

Comment 4

[Huw Lynes]

Sorry the JSerror is a red-herring. It's a problem with my profile. Creating a
fresh profile has removed the JS error but the rendering problem is still there.

Comment 5

[Olivier Cahagne]

confirming 20030819 (only on Linux, not Win2k), happens in latest nightly build
and also GTK2 build.
It'd be nice to have someone testing this on Linux PPC, MacOSX or some other
platform.

Comment 6

[Phil Schwartau]

Confirming with Mozilla trunk build 2003-08-22 on Linux.

As indicated in Comment #1, this is not a JavaScript problem,
but a layout issue. Reassigning to Layout for further triage -

Comment 7

[Phil Schwartau]

Argh, component accidentally changed. Reassigning (again) to Layout.

Comment 8

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

Changing URL from http://crew.tweakers.net/crisp/lemmings/ to
http://crew.tweakers.net/crisp/lemmings/desktop.html which is a much faster way
to see the bug.

Comment 9

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

->views, although maybe widget

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We really need a reduced, non-DHTML testcase for this.

Comment 11

[crisp]

reduced non-dhtml testcase: http://crew.tweakers.net/crisp/lemmings/nodhtml.html

you should see 1 lemming standing in the upperleft corner; if this is the case 
than something is wrong in the dynamic addition of elements and setting a class 
on that element through javascript that has the overflow property set.

Comment 12

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

Attachment: slightly simpler testcase

Comment 13

[crisp]

confirmed with the latest testcase using Mozilla 1.4 on Slackware 9.0; this is 
total unexpected behaviour in my humble opinion and can be classified as a 
major bug.

Comment 14

[crisp]

Attachment: Problem reason explained

Found when the problem occurs.
In the attachment I have linked 2 images within a div with overflow:hidden.
Both these images have width: 256px via the attached class; the first image
however is on disk only 128px width so is being resized; that's when the image
is visible beyond the div. The second image is also 256px on disk, and in this
case the remaining part of the image is not visible beyond the div.

For my Lemmings game I had resized all the images by 50% to save bandwidth;
that's when the error came up.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Almost certainly a GTK image bug. -> tor

Comment 16

[tor]

Attachment: handle clip mask and region for scaled images

This is essentially bug 78497 for the scaled image path.  X doesn't allow us
to set both a set of clip rectangles and a clip mask, so we need to handle
the rectangles by hand.

Comment 17

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We should get this thing reviewed in case we want it for 1.5. It's a pretty bad
bug; I'm surprised it hasn't been reported more.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+        PRUint8 *scaledRGB = (PRUint8 *)nsMemory::Alloc(3*dstWidth*dstHeight);

Need a success check here. out of memory => BOOM

+          if (NS_SUCCEEDED(clipRgn.GetRects(&rectSet))) {

What is this actually supposed to do? it looks like GetRects always succeeds.

+          nsMemory::Free(scaledRGB);

How come this is only called if the CopyRgn succeeded?

Shouldn't you check to see if the clip region is really clipping the target
rectangle? Otherwise whenever any clip region is set anywhere, we'll crawl
through the slow path.

Comment 19

[tor]

Attachment: address roc's comments

> +	   PRUint8 *scaledRGB = (PRUint8
*)nsMemory::Alloc(3*dstWidth*dstHeight); 
> 
> Need a success check here. out of memory => BOOM

Added.

> +	     if (NS_SUCCEEDED(clipRgn.GetRects(&rectSet))) {
> 
> What is this actually supposed to do? it looks like GetRects always succeeds.


Test removed.

> +	     nsMemory::Free(scaledRGB);
>
> How come this is only called if the CopyRgn succeeded?

Nesting error.

> Shouldn't you check to see if the clip region is really clipping the target
> rectangle? Otherwise whenever any clip region is set anywhere, we'll crawl
> through the slow path.

By the time we've figured out if the clip region matches the target
rectangle we would end up executing all the overhead of the "slow" path,
since we'll end up with a single rectangle.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 130866 [details] [diff] [review]
address roc's comments

OK, looks good now. Not sure if we should take it for 1.5; this presumably has
been broken for a long time if not forever, and hasn't been a big deal.

Comment 21

[tor]

Checked in on trunk.

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

+        PRUint8 *scaledRGB = (PRUint8 *)nsMemory::Alloc(3*dstWidth*dstHeight);
+
+        if (!scaledRGB)
+          return NS_ERROR_FAILURE;

why not NS_ERROR_OUT_OF_MEMORY?

Comment 23

[tor]

Nit fixed.