390833 - Meebo chat windows are incorrectly rendered and handled in firefox 3 alpha builds

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect, P3)

Description

[Benjamin Xiao]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7) Gecko/2007080210 GranParadiso/3.0a7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7) Gecko/2007080210 GranParadiso/3.0a7

The meebo chat windows are not rendered properly. They look like they have whole chunks missing out of the sides of the ajax windows. Also when these windows are closed, the mouse seems to be stuck in click mode and any movement of the mouse will begin to highlight the rest of the page. Clicking again gets rid of it however. This bug has been present in all the firefox 3.0 builds.

Reproducible: Always

Steps to Reproduce:
1.Login to meebo
2.Launch a chat window.
3.Close chat window.
Actual Results:  
Chat windows rendered improperly and mouse acts weirdly when chat windows are closed.

Expected Results:  
Chat windows do not have chunks missing from the sides and closing the windows should not cause the mouse to highlight the rest of the page.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

--> core::layout, blocking-1.9?

Comment 2

[Eli Friedman]

The issue here appears to be bug 225548.  (I kind of thought we were past the days of table-based markup, but I guess not.)

Comment 3

[Benjamin Xiao]

Are you sure? I've tried resizing the width of my browser window as described in bug 225548, but it doesn't seem to be changing anything.

Comment 4

[Eli Friedman]

See bug 225548 comment 19 and the following comments.

Comment 5

[Benjamin Xiao]

Ah I see, in Testcase #2 the mozilla logo is not stretched out to fill the box and the borders aren't rendered properly.

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: screenshot (after patch from 385823)

The image resizing bug is (tentatively) fixed by my "patch v2" for bug 385823.

2 meebo bugs remain, though:
 1. the weird "select-all" behavior on mouse movements after an IM sub-window closes.
 2. (new) Small black lines appear in some meebo sub-windows -- see attached screenshot.  These lines disappear when the sub-window is dragged around, but they reappear when the sub-window loses focus.  There's an even more obvious black line in the upper-left corner of the Buddy List window. (not shown in the screenshot, so as to protect the identities of my buddies. :))

Comment 7

[Benjamin Xiao]

when is this patch gonna make it into the nightlies? Its been two months. :P

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: screenshot (before patch from 385823)

Comment 9

[Daniel Holbert [:dholbert]]

Benjamin: That patch hasn't been checked in yet because, after it was reviewed and the review comments were addressed, we were in beta-lockdown phase, with only a select number of patches being approved for checkin.

I just requested approvalM9 for the patch, and if that's approved, I'll be able to check it in for Firefox3, beta 1.  

If not, I'll make sure it's checked in right after we re-open the tree, and it'll make it into nightlies right after the beta release.

Comment 10

[Daniel Holbert [:dholbert]]

approvalM9 was approved, so I just checked in the patch for bug 385823.

Leaving this bug open for the other Meebo issues mentioned in Comment 6.

Comment 11

[Benjamin Xiao]

I see the fix in the latest nightlies, great work btw. I don't see the black lines that you describe but rather transparent lines, where the lines are actually chunks missing from the window. You can tell because parts of the background and text show through.

I found another bug. You can no longer pop up the sub windows into a separate firefox window. When you click on the button in the top right corner, Firefox briefly pops up a window and then immediately closes it again.

Comment 12

[Martijn Wargers (dead)]

(In reply to comment #11)
> I found another bug. You can no longer pop up the sub windows into a separate
> firefox window. When you click on the button in the top right corner, Firefox
> briefly pops up a window and then immediately closes it again.

Thanks, I filed bug 403005 for that.

Comment 13

[Martijn Wargers (dead)]

I think the weird "Select-All" behavior mentioned in comment 6 might be fixed by the patch in bug 401528.

Comment 14

[Martijn Wargers (dead)]

Attachment: unminimized, zipped testcase

I'm trying to minimize of what I think what the second issue is in comment 6.
This is an unminized work-in-progress testcase.

Comment 15

[Martijn Wargers (dead)]

Attachment: testcase

Ok, I minimized it to this.
This shows a smallish red block at the right in current trunk build, which it shouldn't.
Note that the table has border-collapse: collapse; set.

Comment 16

[Benjamin Xiao]

I tried the test case. What's weird is that when I scroll away so that the red block is off the window, and then scroll back towards it, I no longer see the red block. If I switch to another tab when I have it in the window, it reappears. The lines in the meebo windows do the same thing. If I move them around a bit some of the lines disappear and reappear again if I switch to another tab and then back again.

Comment 17

[Daniel Holbert [:dholbert]]

Attachment: screenshot (testcase, linux trunk)

I don't see any red in the testcase, but I do see a too-thick border on the right side.   Here's a screenshot of what I see.

I can make the thick border briefly look thin again by scrolling it out of view and back into view, or by slowly resizing the window to hide the border and then show it again.

Comment 18

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (with 1x2 px image)

This reduced testcase has a red (on Windows) or black (on Linux) band at the very right edge of the green block.

This has something to do with the scaled image's dimensions, 1px wide by 2px high. See upcoming reference case that only differs in that it uses a 1px-square image.

Comment 19

[Daniel Holbert [:dholbert]]

Attachment: reference 2 (with 1x1 px image)

Same as testcase 2, except that this one uses 1x1 px image, and doesn't show the bug as a result.

Comment 20

[Daniel Holbert [:dholbert]]

Notes on testcase and reference case 2:
 - The discolored band is present at some zoom levels but not at others.
 - The discolored band is a region that should be green (not white).  You can see this from comparing zoomed views of test vs. reference it's present,  that 

Seems like an invalidation bug on the image region, given that hiding and re-showing the band can temporarily fix the problem (as described in comment 16 and comment 17).

Since the only difference between testcase 2 and reference 2 is the original dimensions of the image, this seems like an issue with image scaling.... maybe a cairo bug?

CC'ing Stuart and Vlad, as they were involved with the similar Bug 377634 (which I think was fixed by Bug 384681).

Comment 21

[Daniel Holbert [:dholbert]]

(In reply to comment #20)
> see this from comparing zoomed views of test vs. reference it's present,  that 

s/"it's present,  that"/". dholbert needs to edit his bugzilla comments better"/

Comment 22

[Daniel Holbert [:dholbert]]

Vlad suggested looking at what gets passed into nsLayoutUtils::DrawImage, to see if there are any dimensional differences in testcase 2 vs reference 2.

However, it looks like they're getting the same dimensions in both cases:
aDestRect:   {x = 60, y = 60, width = 23880, height = 6000}
aDirtyRect:  {x = 60, y = 60, width = 23880, height = 6000}
aSourceRect: 0x0

In pixels, that is:
  x = 1px
  y = 1px
  width = 398px   ( = 400px - 1px td-border on each side)
  height = 100px

So, those look like the right dimensions.

Comment 23

[Daniel Holbert [:dholbert]]

Attachment: testcase 3 (with 1x2 px image)

Here's a much-reduced form of testcase 2 -- just the scaled image, with no containers aside from the <body> element.

Comment 24

[Daniel Holbert [:dholbert]]

Attachment: reference 3a (with 1x1 px image)

Comment 25

[Daniel Holbert [:dholbert]]

Attachment: reference 3b (with 2x2 px image)

I'm including this 2x2 px reference, too, because the 1x1 px reference actually triggers a special-case code path in nsThebesImage::Draw

 396     if (mSinglePixel) {
 ...
 406         return NS_OK;
 407     }

Comment 26

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I can no longer reproduce the bugs on meebo, tested on both Win32 and OSX.  Note that I recently broke the 2x2 testcase here too, because images that have a solid color throughout are going to trigger the same optimization.  If we need another testcase here, it's gotta have at least 1 differing pixel.

I'm going to resolve this as WORKSFORME; please reopen if this is still an issue.

Comment 27

[Martijn Wargers (dead)]

I'm still seeing this with current trunk build on windows.
With my testcase, I still see the red box at the right as mentioned in comment 15, which I shouldn't see.
Also, I see a difference between "testcase 2 (with 1x2 px image)" and " reference 2 (with 1x1 px image)" which should look the same. I see small red line at the right, with the reference.
Also, I see a 1px difference between testcase3 and the references (the references seems 1px wider).

Comment 28

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yeah, testcase 2 I see red on win32.  Ok.  Note that it's actually easier to see what's going on if the body background is set to black, because the 1px white border then shows up.

So, from the log on win32:

Fill the background with white (I think this is the standard fill-with-opaque-for-content -- oddly enough, if I change the background color to black, I don't see this paint -- may be worth optimizing out):

0[c063f0]: ## 35f6b38 nsTRC::Translate 0 0
0[c063f0]: ## 35f6b38 nsTRC::SetColor 0xffffffff
0[c063f0]: ## 35f6b38 nsTRC::FillRect [0,0,60000,18000]
0[c063f0]: ## 35f6b38 nsTRC::FillRect raw [0.000000,0.000000,1000.000000,300.000000]

Fill the background with the actual bg color:

0[c063f0]: ## 35f6b38 nsTRC::PushState
0[c063f0]: ## 35f6b38 nsTRC::SetClipRect [0,0,60000,18000] 0
0[c063f0]: ## 35f6b38 nsTRC::SetColor 0xffffffff
0[c063f0]: ## 35f6b38 nsTRC::FillRect [0,0,60000,18000]
0[c063f0]: ## 35f6b38 nsTRC::FillRect raw [0.000000,0.000000,1000.000000,300.000000]

Draw the red background:

0[c063f0]: ## 35f6b38 nsTRC::Translate 60 60
0[c063f0]: ## 35f6b38 nsTRC::SetColor 0xff0000ff -- RED
0[c063f0]: ## 35f6b38 nsTRC::FillRect [0,0,23940,6060]
0[c063f0]: ## 35f6b38 nsTRC::FillRect raw [0.000000,0.000000,399.000000,101.000000]
0[c063f0]: ## 35f6b38 nsTRC::Translate -60 -60

Now draw the white table borders:

0[c063f0]: ## 35f6b38 nsTRC::SetColor 0xffffffff
0[c063f0]: ## 35f6b38 nsTRC::FillRect [0,0,60,6060]
0[c063f0]: ## 35f6b38 nsTRC::FillRect raw [0.000000,0.000000,1.000000,101.000000]
0[c063f0]: ## 35f6b38 nsTRC::SetColor 0xffffffff
0[c063f0]: ## 35f6b38 nsTRC::FillRect [23940,0,60,6060]
0[c063f0]: ## 35f6b38 nsTRC::FillRect raw [399.000000,0.000000,1.000000,101.000000]
0[c063f0]: ## 35f6b38 nsTRC::SetColor 0xffffffff
0[c063f0]: ## 35f6b38 nsTRC::FillRect [60,0,23880,60]
0[c063f0]: ## 35f6b38 nsTRC::FillRect raw [1.000000,0.000000,398.000000,1.000000]
0[c063f0]: ## 35f6b38 nsTRC::SetColor 0xffffffff
0[c063f0]: ## 35f6b38 nsTRC::FillRect [0,6060,24000,60]
0[c063f0]: ## 35f6b38 nsTRC::FillRect raw [0.000000,101.000000,400.000000,1.000000]

Then draw the image contents:

0[c063f0]: ## 35f0fd0 nsThebesImage::Draw ctx 35f6b38
   src [0.000000 0.000000 1.000000 2.000000]
  dest [1.000000 1.000000 398.000000 100.000000]
 trans [0.000000 0.000000]
 pixel: 0


going back up to nsLayoutUtils::DrawImage, the DestRect in original appunits is:

  aDestRect: [60 60 23880 6000]

So layout is asking for a red rect 23940 twips in width (399 pixels), and wants the image drawn with a width of 23880 twips (398 pixels).  gfx is doing exactly that, I think.  But... now I'm not at all sure why I don't see this problem on MacOS X, since the trace numbers look identical.  I see it with linux and win32, on testcase 2; with the further same-color optimization, the problem goes away with testcase 3, but only because of that.

Comment 29

[Benjamin Xiao]

I no longer see this issue with Meebo in win32. All of the test cases in this bug work as well. I am setting this as WORKSFORME.