User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070208 Minefield/3.0a3pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a3pre) Gecko/20070208 Minefield/3.0a3pre When using multiple tabs, a strange line shows up in every lower corner of a tab (except for the tab on the foreground). Reproducible: Always Steps to Reproduce: 1. Just open two or more tabs Actual Results: A strange line shows up in the lower corner of the tab. Expected Results: This line shouldn't be there

I'll check the latest bits, but in my Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070204 Minefield/3.0a2pre build I am not seeing it. but, this looks like something we saw when working on the tab strip / theme refress back for fx 2.0 bert, anything special about your display or dpi settings?

ok, I see this in the latest trunk bits, but I also see everything in serious zoom.

> ok, I see this in the latest trunk bits ok, now i see it. I think might be fall out related to #177805. > but I also see everything in serious zoom. that was a mix of my layout.css.dpi being set to 300 and the landing of bug #177805. as for my why dpi was set so high, must have been from when working on the fx theme for 2.0 for large dpi issues. thanks to gavin for pointing me at bug #177805

I wonder if this is really bug #350686, just exposed now due to bug #350686 as for my 300 dpi oddness, see bug #350686

That could be the same effect as seen in bug 369864, just vertically; although I don't yet understand how the vertical alignment for the tabs is defined.

(In reply to comment #2) > bert, anything special about your display or dpi settings? I don't think so. I didn't change any dpi setting, and I'm running on a quite normal 1280x1024 resolution. Maybe this problem doesn't show up on the next release, it might even be a strange coincidence, we'll see.

I see this too in the latest nightly. WinXP SP2, no changes made into display settings.

workaround: .tabbrowser-tab { margin-bottom: 1px !important; }

on a related note, try this: toggle between two blank tabs and the favicon for a blank document will jump by 1 px. looking into dao's workaround.

dao's work around fixes this bug, and my related issue in comment #12 and achieves parity with fx 2.0. I'll work it into our theme css and attach for review.

there's probably a core bug that needs to be fixed, and a workaround for this particular case should only be the last resort.

I agree with Dão. As far as I can see; the tab "content" (favicon and title) is too small. If you select a tab, the "content" moves down 1px, and the line is hidden under the content.

Seth, I hope you agree on that: Adding hacks for all manifestations of the same bug isn't a solution. The location bar, its favicon, and the search bar aren't centered either (1px too low); and that are only those that are obvious to me. -> updating the summary and moving to Core

dao: yes, you are right, this is core bug. thanks for re-assigning it.

Are all the elements where this is showing up elements with rounded corners? (That is, some -moz-border-radius-* nonzero?) This seems pretty similar / the same as bug 353860.

(In reply to comment #21) > Are all the elements where this is showing up elements with rounded corners? > (That is, some -moz-border-radius-* nonzero?) To my knowledge, no. For example, the favicon doesn't seem to be centered within the urlbar. (Contrary to comment 16, the urlbar itself seems to be centered, although it looks misaligned due to an odd line (not a border) at the bottom of the toolbar; see attachment 258373 [details].)

So has anyone managed to create a minimal-ish testcase for this, by chance?

The summary isn't quite accurate: the image element is centered, but the raw image is misaligned. Also, I can't reproduce this without <deck>.

So the heart of the matter for that testcase seems to be that when we paint images we snap the rect to pixel boundaries (rounding to the nearest pixel) before calling down into thebes -- the rounding happens in nsLayoutUtils::DrawImage. Then we're a single-pixel image, so nsThebesImage::Draw does a Rectangle() at (0,1), which are the rounded coordinates) of the given size. On the other hand, nsCSSRendering::PaintBackgroundColor just does a FillRect on the fractional-pixel rect. nsThebesRenderingContext::FillRect calls Rectangle() on a rectangle at (0,0.5), with snapToPixels set to true. This uses gfxContext::UserToDevicePixelSnapped, which also seems to round to nearest pixel... but the numbers it tries to round are the output of UserToDevice. And the cairo transform has a -0.5px vertical translation. So UserToDevice() hands back the top-left vertex as being at (0,0). Rounded, this gives (0,0). If you recall, the image case was doing Rectangle() at (0,1). Which gives (0,0.5) after calling UserToDevice, then gets rounded to (0,1). So you see a 1px offset. I'm not sure whether the right solution is to stop rounding in nsLayoutUtils::DrawImage (since the numbers we're rounding there have nothing to do with actual device pixel offsets), to continue rounding there but take into account the rendering context transform, or something else. I do find it odd that we end up with the fractional-pixel background fill somehow at (0,0) before rounding; I wonder what coordinate system that thinks it's in.

Based on the rules I test in layout/reftests/pixel-rounding/, the background is wrong (we should round edges at half-pixels to right or bottom); not sure why none of the tests there are failing, though.

Actually, I take that back -- the background is correct, and for the image, we're somehow rounding the 0.5 up to 2.

The <deck> in the testcase seems to be key, though. That implies a view (+ widget?).

This fixes it for me. If I didn't remove the pixel snapping in nsThebesImage::Draw, I get blurry images when they're half-pixel aligned. I don't think that's needed anymore for the layout callers going through nsLayoutUtils::DrawImage; not sure if there are others we need to care about. (But, really, it ought to be able to pixel-snap correctly. Maybe I should figure out how to fix it instead...)

And I get a REFTEST UNEXPECTED PASS for layout/reftests/pixel-rounding/offscreen-image-pos-5.html , which is bug 368280.

Comment on attachment 264806 [details] [diff] [review] patch (checked in) I think that's the main caller, so as long as all works fine there, this should be ok.

Checked in to trunk.

This patch fixes the testcase, but I can still reproduce the original bug. I'm working on a better fix. (I think reftests can't catch this bug because the way they paint bypasses the standard paint code.)

Never mind about having a fix for this... the problem here is the tabs, and that's likely a border-radius bug. (I think I have a fix for Bug 380438, though...)

(In reply to comment #35) > the problem here is the tabs, and that's likely a border-radius bug. -moz-border-radius is specified for .tab-image-left and tab-image-right but not for .tab-image-middle and .tab-close-button or .tabbrowser-tab. Resetting these radii via DOMi makes the horizontal lines disappear, but the tab remains misaligned.

(In reply to comment #34) > This patch fixes the testcase, but I can still reproduce the original bug. It did fix the favicon too.

I'm not sure if this is actually causing the tabs bug. At least, I got to this testcase by reducing a <tabbox> with all its anonymous content and a misaligned tab.

(In reply to comment #36) > (In reply to comment #35) > > the problem here is the tabs, and that's likely a border-radius bug. > > -moz-border-radius is specified for .tab-image-left and tab-image-right but not > for .tab-image-middle and .tab-close-button or .tabbrowser-tab. That was wrong for .tabbrowser-tab, which does have a border-radius (originating from a |tab| rule), but removing it doesn't seem to make any difference.

(In reply to comment #38) > Created an attachment (id=264943) [details] > testcase 2 > > I'm not sure if this is actually causing the tabs bug. I am now, so let me clarify the previous comments. Comment 36 is the crux -- there are two remaining bugs: 1) the horizontal lines in .tab-image-left and .tab-image-right, caused by border-radius, and 2) the vertical misalignment (see the leftmost circle in attachment 254550 [details]). Testcase 2 represents 1), which is likely the same underlying problem as bug 353860. I suggest to focus on 2) here.

So both testcases now work fine, but the overall 1px too low problem still isn't fixed. I'm not really sure what's going on; now the tab background area is still 1px too low (the close box looks like it's in the right spot; that is, it looks like it's 1px "too high"). But there is no more bogus horizontal line/blurryness that was due to borders and border radius.

So I may need Eli's help here -- the problem is that for the middle part of the tabs, we get into nsThebesRenderingContext::DrawTile with a Y coordinate of 1590 twips, which is 1590/60 = 26.5. We then end up drawing at 26.5, which causes the problem. If we round, we'd draw at 27, which is also visually wrong. If I add a floor() to both the x/y offset (phase) and the target rect, I get the attached screenshot. The middle portions look visually correct, the left/right sides are still wrong, presumably because they're not going through DrawTile. Where does this error lie? Is it an error that we're passing non-pixel-aligned data down to nsThebesRenderingContext?

Hmm, actually, the tabs seem to look fine in my tree. Let me get my view fixes into the tree, and then I'll get back to you.

k, I'll wait until you do that

punting remaining a6 bugs to b1, all of these shipped in a5, so we're at least no worse off by doing so.

Can someone illustrate a screenshot with the issue using a build from today? I'm not quite sure what the issue is.

You can see the problem if you look at the bottom of the tabstrip. The entire 1px "stripe" is missing from the button on trunk (since the entire tabstrip is lowered by 1px).

Oh; I see... On my computer Fx 2.0 looks much closer to the screenshot you labeled "trunk" than the one you labeled "branch" :(

(In reply to comment #10) > workaround: > > .tabbrowser-tab { > margin-bottom: 1px !important; > } After talking with Eli, we don't think this is a bug -- our rounding is pretty consistent right now with everything other than native theme code (and that's mostly getting fixed). The problem seems to have been that Fx2 just happened to round the other way for this situation; he says that he can get Fx2 to look like the current trunk when he runs with a higher dpi setting, which causes differences in rounding on Fx2. So, this needs to be fixed in the theme -- either with the above workaround, or with some better analysis about how those pieces work together. Better yet, the tabs should be redone so that they use the new border-image support being done in bug 378217, because that's bound to be faster than the way we're building the tabs up now.

Comment on attachment 272943 [details] [diff] [review] patch for winstripe tabs (checked in) Per discussion with Vlad over IRC, we're going to hold off on this in favor of the more structural changes to the tabstrip coming with bugs 387345 and 339964.

There's no reason why this regression shouldn't be fixed before any other work starts. The current stylesheet is broken -- it contains margin-bottom: 1px, which hasn't any effect because of margin: 0px !important.

Comment on attachment 272943 [details] [diff] [review] patch for winstripe tabs (checked in) Since this has also caused bug 388076, I'm re-requesting review. Again, I think this should be fixed as soon as possible so that a refactoring of the tab strip has a working starting point.

I'm not the right person to approve this -- I'd try either mconnor or maybe gavin? (Sorry for the runaround, I just don't touch any fo the theme stuff..)

Gavin, why in-testsuite-? It seems worth it to me to add some testcases for this situation in general.

(In reply to comment #57) > Gavin, why in-testsuite-? It seems worth it to me to add some testcases for > this situation in general. What situation in general? Testing that our rounding behavior doesn't change? That seems like a test that would belong in the bug that changed our rounding, rather than the bug about fixing the theme to compensate, but I'm not too picky (and realize that "that bug" might have been "switch to cairo" or "Eli's unit patch" or some other large change that's hard to isolate). Either way, I don't really know how you'd test this; feel free to reset to "?" if you have ideas :)

Checking in toolkit/themes/winstripe/global/browser.css; /cvsroot/mozilla/toolkit/themes/winstripe/global/browser.css,v <-- browser.css new revision: 1.31; previous revision: 1.30 done

If I knew how to test, I would have attached a test...

Checking in browser/themes/winstripe/browser/browser.css; /cvsroot/mozilla/browser/themes/winstripe/browser/browser.css,v <-- browser.css new revision: 1.96; previous revision: 1.95 done