37589 - Strange dots next to Slashdot news headers

Status: RESOLVED FIXED

(Core :: Layout: Images, Video, and HTML Frames, defect)

Description

[Blake Ross]

Build ID: 2000042808

There seems to be a strange rectangular dot preceding each news header on /.  
It's not only on the same page, but is also to the left of the headers in 
anactual story/comments page (which you can reach by clicking one of the news 
stories for the day).

Haven't had a chance to study this yet, but figured I should report it since 
the dots don't appear in IE 5.5 or NS 4.72

Comment 1

[Jesse Ruderman]

Attachment: html code that demonstrates the problem

Comment 2

[Jesse Ruderman]

This was reported previously as bug 18657, but that got duped and then its dup-
of got marked as WORKSFORME.

The offending image is http://images.slashdot.org/slc.gif loaded on any dark 
background.  The bottom quarter of the image looks different each time mozilla 
renders it.  If you load the image from the same server without restarting 
mozilla, you will see the same corruption each time.  If you trick mozilla into 
thinking it's using a different server (as I did by varying the case in the 
hostname), however, it will corrupt each one differently.

Trying imagelib component.

Comment 3

[troy]

Reassigning  to component owner

Comment 4

[R.K.Aa.]

dang..i just commented about this in bug 36869 - this seems to be a dupe of it.
Can it be the alt-tag that displays in that cell, just underneath the image?
The screenshot in 36869 isn't too clear but indicates it can actually be "" that
displays, the first " in bold, and both tainted in shades of blue?
I can't reproduce this bug on Linux, thus the guesswork.

Comment 5

[Jesse Ruderman]

Attachment: screenshot of testcase (2000042808, win 98)

Comment 6

[pnunn]

weird.
-p

Comment 7

[pnunn]

It has nothing to do with the table. It appears to
be solely an image problem (or an image rendering problem).
(Note to self: see local test file.)

Comment 8

[Paul McGarry]

I just added some comments to bug 38656 that are pertinant to this. 

Comment 9

[WD]

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

Comment 10

[WD]

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

Comment 11

[dcone (gone)]

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

Comment 12

[pnunn]

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

Comment 13

[pnunn]

Attachment: another demo img of bug from dupe#42285

Comment 14

[Alan S. Jones]

From 42285 I have created a smaller test case with and without the image that 
causes the problem.

http://users.ipa.net/~asj/test/slash.htm

Comment 15

[R.K.Aa.]

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

Comment 16

[Gagan]

seems to be only on specific images... 

Comment 17

[pnunn]

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

Comment 18

[Hixie (not reading bugmail)]

*** This bug has been marked as a duplicate of 18754 ***

Comment 19

[Hixie (not reading bugmail)]

I don't believe I just did that. This is the second time I've accidentally
marked a bug duplicate in two days. I offer you my humble appologies once more.

Comment 20

[Arne de Bruijn]

Attachment: Patch (moves gif_clear_screen after converter init)

Comment 21

[Arne de Bruijn]

The problem is caused by a bug in the gif decoder: it calls gif_clear_screen 
before calling ImgDCBSetupColorspaceConverter. Then the target image is written 
to as if it was an 8 bit image, while it is actually 24 bit, leaving 
uninitialized garbage in the target image.

The attached patch solves this by moving the gif_clear_screen call to after the 
ImgDCBSetupColorspaceConverter call. This undoes the fix for bug #13048, which 
moved gif_clear_screen to use the is_transparent flag. Therefore the 
is_transparent flag clearing is moved too, to reset it after the image is 
done instead of before the image is decoded. This seems the right place since it 
is set between the decoding of the last and the next image... 
(probably it would be even better to split the gif_image_start state in two 
states: an initializing state that resets all these flags, and then jumps to the 
looping state, but I didn't dare to change so much.)

Unfortunately the external accessible page for bug 13048 seems to be changed so 
I can't test if bug 13048 is still fixed :-(

Comment 22

[Jesse Ruderman]

Adding patch keyword.  Thanks :)

It looks like there's a copy of the page for bug 13048 at an internal Netscape 
url, so someone at Netscape should be able to test whether your patch leaves 
that image intact.

Comment 23

[Jason Eager]

Adding review, approval, rtm keywords. This seems to be a simple straightforward
change, and we should be able to do this correctly in our release.

Comment 24

[Jerry Baker]

Either there is something funky with that image, or Paint Shop Pro 7 has the 
exact same bug as Mozilla. When I view the gif in PSP7 and do Colors -> Show 
Palette Transparency, that dot is the only thing left. It is quite odd. If 
Mozilla can fix that, that would be nice though.

Comment 25

[Andrew Simmonds]

Happens here in Windows 2000
kinda annoying

Comment 26

[pnunn]

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

Comment 27

[pnunn]

Attachment: resized problem gif

Comment 28

[pnunn]

The image has 2 colors that should be set for transparency.
The colors are close (70,82,84 and 50,102,100).

This doesn't explain the problem yet. The gif in question show
band across 2/3 of the bottom of the image, not all the way across
as the 2nd color would predict.
And there are reports of the band *flashing*. 

I'll attach a screengrab of a testpage showing the image
transparency issue.

-p

Comment 29

[pnunn]

Attachment: scrngrab of test page showing transparency problem.

Comment 30

[selmer (gone)]

Has anyone reviewed the patch?  Is there something wrong with it?  Time is
almost completely gone to get this into rtm, but if this is a correct patch that
can be reviewed and tested _immediately_ it isn't yet too late...

If it isn't worth the bother, please mark rtm- in the whiteboard.

Comment 31

[Phil Peterson]

PDT marking [rtm-]. Looks like this patch has languished without review, and we
can't hold for this bug at this time.

Comment 32

[Arne de Bruijn]

Meanwhile I received the files of bug 13048, and I didn't see anything wrong 
with my patch. I didn't see anything wrong either when I undid the patch of bug 
13048, so maybe I'm missing something, but I don't think that should hold up 
the patch for this bug.

Comment 33

[pnunn]

Looks ok. Thanks for the patch and
sorry it took so long to get it reviewed.

r:pnunn

Comment 34

[tor]

sr=tor

Comment 35

[Blake Ross]

Fix checked in.  Thanks [e-mail] for the patch!

Comment 36

[Terri Preston]

Verified Win build 2001021604
Verified Linux build 2001021608
Verified Mac build 2001021604

Comment 37

[WD]

I still occasionally see this
Re-opening

Comment 38

[WD]

Attachment: Screenshot of Slashdot

Comment 39

[bcubique]

Attachment: screenshot 01/31/2002 Win2000

Comment 40

[Matthew Mastracci]

Seeing this in 2002020103, Windows XP with nVidia GeForce 2 card

Comment 41

[Ken Weaverling]

I saw this image bug crop up in an unexpected place -- while viewing screenshots
for cygwin/xfree86 rendering a konquerer window displaying slashdot in an 8bpp X
server running within Microsoft Windows (parse that!).

http://xfree86.cygwin.com/screenshots/cygwin-xfree86-8bpp-kde.png

Konq doesn't use gecko, does it? Is it determined that the image itself isn't
the problem? 

fwiw, the same problem still occurs for me in XP on 0.9.8 using a geforce2 gts
card...

Comment 42

[R.K.Aa.]

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

Comment 43

[Hixie (not reading bugmail)]

Original problem was seen at: http://www.slashdot.org/
Changing URL to a data URL testcase.

Comment 44

[Hixie (not reading bugmail)]

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

Comment 45

[Hixie (not reading bugmail)]

See blown up screenshot here: 
   http://bugzilla.mozilla.org/attachment.cgi?id=63726&action=view

Comment 46

[Eric Vaandering (no email)]

A new type of behavior has also appeared for me with 0.9.8 Somtimes there is an
actual white gap, guessing 3-4 pixels high at the bottom of the image. I'll
attach a screenshot next time this happens.

Still see the dots most of the time (again with 0.9.8)

Comment 47

[David Barnes]

I don't see dots, but I do see the gray line at the bottom of the image. It only
seems to happen first time I go to the site after my cache is emptied. If I hit
refresh, the problem goes away.

Comment 48

[Jerry Baker]

Mass removing self from CC list.

Comment 49

[Jerry Baker]

Now I feel sumb because I have to add back. Sorry for the spam.

Comment 50

[Ryan WYler]

If you clear your cache (while not on slashdot) then goto slashdot.org you'll
see the image flaws next to each article header.  As soon as you refresh the
screen, or minimize the window and maximize the window or drag another window on
top of the messed up images.. it fixes its self.

(this is on MacOSX 10.1 with build 2002060203)

Comment 51

[ArronM (:paper)]

I have a fix for this.. taking

Comment 52

[ArronM (:paper)]

Sorry for the delay.  I've been waiting on Bug 87819 (although I can't remember
why, but I'm going to assume it was because it blocks this one)

Testcase: http://www.animecity.nu/mozilla/slashdot.html

Placeholder does not disappear after image is loaded.  Build 2002071908.
Patch coming in the next day or so.

Comment 53

[ArronM (:paper)]

Attachment: good karma coming, mod me up :P

Reviewers reference:

Chunk #1
Update the whole image width, and the right y-axis areas

Chunk #2 and #3
Read the comments in the code :D

Comment 54

[ArronM (:paper)]

futuring since I don't have time to hunt down r='s

Comment 55

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

+      if (imgHeight > realFrameHeight) {
+        PRInt32 imgWidth;
+        decoder->mImageContainer->GetWidth(&imgWidth);
+
+        nsRect r(0, realFrameHeight, imgWidth, imgHeight - realFrameHeight);

what if the imgHeight=frameHeight but frame width is smaller than image width?

or does that already work in our code?

Comment 56

[ArronM (:paper)]

biesi:
Yes, that's what chunk #1 makes sure of.  It notifies observers that the whole
width of the image is available, instead of just the frame's width.

Comment 57

[ArronM (:paper)]

Attachment: The Patch

Fixed spelling error and put spaces between operators.	No code change -- just
formatting.

Comment 58

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

Comment on attachment 102620 [details] [diff] [review]
The Patch

ok... two questions:
in the second part: should the comment also mention the bug#?
and in the third part: would a better place for that code be in FlushImageData?
because that seems to be the function which does the other OnDataAvailable
notifications.

Comment 59

[ArronM (:paper)]

Attachment: The Patch

-Fixed height calculations in chunk 1 (was adding framerect.y to nsRect.height
when that's plainly wrong)
-Bug # added to chunk 2

FlushImageData doesn't know whether it's at the end of the image or not, so
putting chunk 3 there wouldn't work.  This is especially true for interlaced
GIFs, who (may?) reach the end of the frame and then go back up again x more
times.	Better to do it at EndImageFrame where we definitely know there's no
more for the frame.

Comment 60

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

Comment on attachment 102686 [details] [diff] [review]
The Patch

r=biesi

Comment 61

[ArronM (:paper)]

Attachment: The Patch

Had an extra chunk on there that wasn't supposed to be there (wasn't on the
other patches as in junk)

Comment 62

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

Comment on attachment 102709 [details] [diff] [review]
The Patch

oh, right, I wondered about that but I misread that as just a comment addition
and thought that was fine.

anyway, r=biesi here.

Comment 63

[tor]

Comment on attachment 102709 [details] [diff] [review]
The Patch

The two new hunks where you call OnDataAvailable() - they're going to
miss a bit if you have an image where the first frame doesn't touch
the top or bottom, aren't they?  You might want to mention in the
comment that we don't have to worry about right/left fixup because
libpr0n currently only does full lines.

Comment 64

[tor]

Comment on attachment 102709 [details] [diff] [review]
The Patch

Oops, overlooked that both hunk 2&3 are run and are actually doing
something different.

sr=tor

Comment 65

[tor]

Checked in.