Serge Gautherie (:sgautherie) Reporter
	Description • 16 years ago

Work fine:
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081104 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/f923f7a759e2) "21292"
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081104 Minefield/3.1b2pre] (nightly) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/fbae114d6133) "21305"

Regressed:
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081105 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/bf2f8ad8f1ce) "21366"
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081105 Minefield/3.1b2pre] (nightly) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/dcec193ba5d7) "21367"

Regression timeframe:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-11-04+01%3A31%3A47&enddate=2008-11-04+23%3A38%3A24

	Serge Gautherie (:sgautherie) Reporter
	Comment 1 • 16 years ago

Site access needs a free account ... but the affected feature/page needs a level 10 character, which takes days to get :-/

This is an unreduced testcase:
the white mountains are meant to be at the upper right corner...

	Serge Gautherie (:sgautherie) Reporter
	Comment 2 • 16 years ago

Ah ... This bug should have the "security" flag set,
as the "picture" I attached is supposed to be undisclosed in the Shinobi game :-<
(Consider it as private data.)

If someone can set the flag... Thanks.

	cmtalbert
	Comment 3 • 16 years ago

Flag set, not sure if that's a proper use for it or not, but it's easy to unset if I'm wrong.

	Serge Gautherie (:sgautherie) Reporter
	Comment 4 • 16 years ago

(In reply to comment #0)
> [Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081104
> Minefield/3.1b2pre] (nightly) (W2Ksp4)
> (http://hg.mozilla.org/mozilla-central/rev/fbae114d6133) "21305"

Note to self: "21302" !

	Serge Gautherie (:sgautherie) Reporter
	Comment 5 • 16 years ago

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081122 Minefield/3.1b2pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/755f505ee163) "21346"

No bug.

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-11-04+11%3A58%3A50&enddate=2008-11-04+23%3A38%3A24

	Bob Clary [:bc] (inactive)
	Comment 6 • 16 years ago

I marked the attachment as private. Is that sufficient to open the rest of the bug?

	Serge Gautherie (:sgautherie) Reporter
	Comment 7 • 16 years ago

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081122 Minefield/3.1b2pre] (home, optim default) (W2Ksp4)
Bad:
(http://hg.mozilla.org/mozilla-central/rev/e16da35c4223) "21358"
(http://hg.mozilla.org/mozilla-central/rev/49dd935efff3) "21351"
Good:
(http://hg.mozilla.org/mozilla-central/rev/2d35868314cf) "21350"
(http://hg.mozilla.org/mozilla-central/rev/755f505ee163) "21346"

This is bug 458487 !

I suggest this should block 1.9.1b2.

	Daniel Veditz [:dveditz]
	Comment 8 • 16 years ago

(In reply to comment #6)
> I marked the attachment as private. Is that sufficient to open the rest of the
> bug?

Maybe, if the people needing to fix the bug have security-group access. Otherwise you should un-hide the attachment and leave the bug private until it's patched, and then re-hide the attachment before opening the bug after the fix. Or get an admin to delete the attachment. Either use is an abuse of the security-group flags and the site's privacy policy saying everything posted is public, a better approach would be to put the image on a private server (where it can easily be deleted later) or offer to email it as needed.

Since everyone involved with bug 458487 can see private attachments (I think) we can just leave it this way for now.

Note that all the images and css used by the attachment are loaded remotely from shinobi.fr so there's no guarantee it will keep working. For long-term regression tests we can build into the tree we should come up with a reduced self-contained file that demonstrates the problem.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 9 • 16 years ago

What is the actual bug here? Can you attach screenshots showing what it's supposed to look like? Better still reduce the testcase...

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 12 • 16 years ago

(In reply to comment #10)
> Created an attachment (id=349666) [details]
> Image of expected layout

This is what I see on Mac trunk.

	Serge Gautherie (:sgautherie) Reporter
	Comment 13 • 16 years ago

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081123 Minefield/3.1b2pre] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081123 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)

Windows only bug ?

	Martijn Wargers (dead)
	Comment 14 • 16 years ago

I can see the bug with today's trunk build. It looks like a painting bug, when overlaying the image with the context menu, it disappears.
Perhaps related to bug 465799?

	Serge Gautherie (:sgautherie) Reporter
	Comment 15 • 16 years ago

(In reply to comment #14)

Why did you remove bug 458487 dependency ?
It's what caused/triggered this !

> I can see the bug with today's trunk build. It looks like a painting bug, when

On which platform ?

> overlaying the image with the context menu, it disappears.

I can confirm this.
Also, on the website, there is a dynamic tooltip, which does repaint under itself too.

> Perhaps related to bug 465799?

Maybe, though it would be more related to 'border' than to 'background-position'.

	Martijn Wargers (dead)
	Comment 16 • 16 years ago

(In reply to comment #15)
> Why did you remove bug 458487 dependency ?
> It's what caused/triggered this !

Sorry, that didn't happen on purpose.

	Serge Gautherie (:sgautherie) Reporter
	Comment 17 • 16 years ago

|border: solid;| comes from the linked css of the initial page;
it's what triggers the wrong painting.

I left 1 (much bigger) cell in the table, so we can still see something :->

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 18 • 16 years ago

What's the expected rendering of that testcase? Is this Windows only perhaps?

	Serge Gautherie (:sgautherie) Reporter
	Comment 19 • 16 years ago

(In reply to comment #18)
> What's the expected rendering of that testcase?

The expected rendering is the upper left of the expected image.
Basically, the testcase should render the "same" (image) with/without the border style !

> Is this Windows only perhaps?

That's what I asked in comment 13...
Martijn didn't answer on which platform he reproduced the bug.

	Martijn Wargers (dead)
	Comment 20 • 16 years ago

I'm on Windows, yes.

	David Baron :dbaron:
	Comment 21 • 16 years ago

I see this on Windows but not on Linux.

	Serge Gautherie (:sgautherie) Reporter
	Comment 22 • 16 years ago

Not Linux, not MacOSX: Windows only, then.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 25 • 16 years ago

I'm pretty sure this is a gfx/cairo bug. I've stepped through and everything down to and including nsThebesImage looks right. In this testcase, we're simply setting the pattern to the image surface and the userSpaceToImageSpace transform is a plain translation by -11,-11 which is correct. The bug only happens on Windows and it only happens when the top-left of the image is outside the fill area.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 26 • 16 years ago

This testcase is slightly simpler.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 27 • 16 years ago

(In reply to comment #25)
> it only happens when the top-left of the image is
> outside the fill area.

Oops. I mean it only happens when the top-left of the image is in the interior of the fill area.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 28 • 16 years ago

We can trigger the bug using background-position instead of border.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 29 • 16 years ago

This is happening because of a bug here:
http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-win32-surface.c#1217

src_r.x is -1, src_extents.width is 600 *and* it's an uint32_t. So -1 is coerced to unsigned int (i.e., 2^32-1) and then we take that mod 600, which returns 495. Not what we wanted.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 30 • 16 years ago

This fixes it. Jeff, could you upstream this patch or something like it?

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 31 • 16 years ago

Bug 458487 triggered this because, before that change, we normalized tile coordinates in nsCSSRendering::PaintBackground in a way that avoided this bug. I got rid of all that normalization code because it wasn't needed, but that allowed this cairo bug to be exposed.

	Jeff Muizelaar [:jrmuizel]
	Comment 33 • 16 years ago

Comment on attachment 351353 [details] [diff] [review]
fix

Looks good to me. I'll upstream it.

	Serge Gautherie (:sgautherie) Reporter
	Comment 34 • 16 years ago

roc, what does "needs landing" whiteboard mean ?
Should it be "checkin-needed" keyword ? Does it mean to wait for update from upstream ? ...

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 35 • 16 years ago

It meas the patch needs to be landed. It doesn't mean the same as checkin-needed, which means "please land this patch for me".

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 36 • 16 years ago

Pushed eb91ec5673df

	Alfred Kayser
	Comment 37 • 16 years ago

Note, the commit message in HG has the wrong bug number:
"Bug 466268. Fix cairo-win32 bug that needed MOD but was using the C % operator. r=jmuizelaar"

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 38 • 16 years ago

Pushed af7b8328c34a to 1.9.1

	Serge Gautherie (:sgautherie) Reporter
	Comment 39 • 16 years ago

(In reply to comment #35)

Ah, noted :-|

*****

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081212 Minefield/3.2a1pre] (nightly) (W2Ksp4)

V.Fixed

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20081212 SeaMonkey/2.0a3pre] (comm-central-win32/1229097559) (W2Ksp4)
(... + http://hg.mozilla.org/releases/mozilla-1.9.1/rev/b4f285d733b6)

verified1.9.1