While fixing bug 305375 (Toolkit; SeaMonkey bug 305384), I tried to work around a bug in sizeToContent() that sometimes causes a white 1-pixel line to the right of the window content. It didn't show up in my tests, but that workaround messed up the alerts size on Windows (see bug 310226). The changes were backed out of the toolkit, but not (yet) out of SeaMonkey. I'm going to test how the original sizeToContent() problem can be reproduced reliably, and then try to come up with a workaround that doesn't have any side-effects.

Scott, can you review this? The new approach is way more selective, only decrementing the window width if the mismatch is exactly one pixel.

Neil, can you r/sr this for SeaMonkey?

I was able to reproduce the original problem with a simple, completely empty alert, like this: Components.classes['@mozilla.org/alerts-service;1'].getService(Components.interfaces.nsIAlertsService).showAlertNotification("", "", "", false, "", null) ; (use in JS console, for example) I strongly suspect this is somehow theme-dependent - in Firefox 1.5b1, the gap is visible, whereas in Thunderbird 1.5b1 it is not. The patches above fix the issue in Firefox reliably, and don't affect Thunderbird at all (because the size of the alert box and the window match in the first place). I'm sure this cannot reintroduce the regression seen in bug 310226.

Oh, and another note: the problem is not a regression, it also appears in Firefox 0.9.3 (not neccessarily introduced at that time, I just had that build lying around), using the same test as above.

Comment on attachment 198661 [details] [diff] [review] fix 1-pixel gap (xpfe) If it's just the width that's wrong, then how about this: window.innerWidth = contentDim.width;

(In reply to comment #5) > If it's just the width that's wrong, then how about this: > window.innerWidth = contentDim.width; I'd like to keep the 1-pixel-off check to not risk a regression: The screenshot in bug 310226 looks like it has both wrong height and width.

(In reply to comment #6) >I'd like to keep the 1-pixel-off check to not risk a regression: >The screenshot in bug 310226 looks like it has both wrong height and width. Ah, but is that because you used resizeTo, which sets the outerWidth?

(In reply to comment #7) > Ah, but is that because you used resizeTo, which sets the outerWidth? Probably, but that's not the whole truth. As Scott wrote in bug 310226 comment 11, setting window.innerWidth = contentDim.width still produces the wrong width.

Jens, are these two patches still valid/something we should check into the trunk? I've lost track of where this issue stands.

(In reply to comment #9) > Jens, are these two patches still valid/something we should check into the > trunk? I've lost track of where this issue stands. The patches wait for review (the toolkit one for yours, to be exact *g*) - after that, they're ready for checkin: From my testing, they fix the issue without side-effects. I'd even nominate them for 1.5, but I guess we're too late in the cycle for that.

Seems this never got checked in...

mozilla/xpfe/components/alerts/resources/content/alert.js 1.7 mozilla/toolkit/components/alerts/resources/content/alert.js 1.7

Comment on attachment 198660 [details] [diff] [review] fix 1-pixel gap (toolkit) Can we land this on the 1.8.1 branch? It's a simple and safe JS fix that has baked on the trunk for two months now.

Comment on attachment 198661 [details] [diff] [review] fix 1-pixel gap (xpfe) Can we land this on the 1.8.1 branch? It's a simple and safe JS fix that has baked on the trunk for two months now.

Comment on attachment 198661 [details] [diff] [review] fix 1-pixel gap (xpfe) Gavin, could you check in the xpfe patch on the branch? Thanks.

Sure, I'll do so later today.

mozilla/toolkit/components/alerts/resources/content/alert.js 1.4.10.3 mozilla/xpfe/components/alerts/resources/content/alert.js 1.5.20.2