This is a follow-up of bug 859751, where I removed this change due to mochitest-chrome test failures. We still want the reduced padding, so we'll deal with that separately here.

Neil, I tried to fix the tests here, as I think a reduction of the padding on Windows should be possible without test failures, but I got stuck at trying to understand what's going on. The objective is to change the padding defined at http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/popup.css#78 to 4px, instead of 10px. The arrowpanel test fails at http://mxr.mozilla.org/mozilla-central/source/toolkit/content/tests/chrome/test_arrowpanel.xul#80, for example with the messages 'anchored on right - got 498, expected 490' and 'anchored on left - got 2, expected 10'. As a test, when I reduce all the panel offsets with 6px as well to 14px - starting from http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/popup.css#50 - the tests do pass, but the panel positioning is off. I'd like to understand where the 8px difference is coming from and how I can incorporate that in test logic. My hope is that this is a 'duh' thing for you, but I just couldn't see the light after staring at the issue for a while... Can you tell me?

The black boders are a bit ugly too. I don't see them on the Win 8 mockup.

(In reply to Guillaume C. [:ge3k0s] from comment #2) > The black boders are a bit ugly too. I don't see them on the Win 8 mockup. Black borders ?

(In reply to Tim Nguyen [:ntim] from comment #3) > (In reply to Guillaume C. [:ge3k0s] from comment #2) > > The black boders are a bit ugly too. I don't see them on the Win 8 mockup. > > Black borders ? I saw them on a Win 7 classic screenshot, but in fact it was normal and it's perfectly fine on Win 7 aero.

> I'd like to understand where the 8px difference is coming from and how I can > incorporate that in test logic. The test is asking the panel to be aligned along the bottom edge and along the left side of the anchor 'bottomright' which appears in the lower-right corner of the test page. The test is expecting the popup to be large enough that it won't fit either horizontally or vertically, so that the panel appears flipped in both directions, instead aligning along the top edge and along the right side of the anchor. However, as you decreased the padding on the popup, the popup is now not as wide, so it no longer flips horizontally. I'd suggest just making the content of the popup in the test a bit bigger such that it is 12 pixels wider. I just did a test on Windows by adding an extra period to the label and the test worked again. Unfortunately, this test is quite finicky though, so that might break on another platform, but if so, you could set the label differently on Windows.

AHA! Neil, thanks so much for that info, it makes complete sense to me now!

Neil, would giving the label and button inside the <panel> a fixed width not make the test less finicky? Or would it defeat the purpose of this test altogether?

Oh, btw, after testing with a longer label text, some assertions indeed fail on OSX with off-by-1px errors. So I _guess_ there's no other way than to only add more characters on Windows.

You could set a specific width as well. The issue here is that arrow panels are styled a bit differently on each platform and the test relies on too precise a placement. Ideally, the test needs to be reworked to be less dependent on this. Does the patch in bug 941051 help?

Alright, I'll try! In the meantime, can you land the patch in bug 941051 on fx-team?

Looks like Neil's updates to the chrome test solved the issue for me, double-checking with a Try push: https://tbpl.mozilla.org/?tree=Try&rev=45389bff184b

Comment on attachment 8369452 [details] [diff] [review] Patch v1: adjust popup padding to 4px on Windows bah, now it's test_popupanchor.xul giving a party :/

Another try run, now only changed the padding, not the arrow positions. https://tbpl.mozilla.org/?tree=Try&rev=d3e14889a1c4

Thanks! remote: https://hg.mozilla.org/integration/fx-team/rev/eae472070610

I may have wrongly understood the purpose of this bug but I don't see the reduced padding in latest Fx-Team build on Win7.

I think this broke the Sync promo: http://cl.ly/image/2A0g0Y2O410j

Ok now I see the change, but I don't understand why the padding of the menu panel hasn't been reduced too.

Stephen, I think you're right. The fix should be simple, but will be dealt with in another bug.

Comment on attachment 8370794 [details] [diff] [review] Patch v1.1: adjust popup padding to 4px on Windows [Approval Request Comment] Bug caused by (feature/regressing bug #): n/a User impact if declined: invalid amount of spacing around the panel edges on Windows. Testing completed (on m-c, etc.): landed on m-c, passed all tests. Risk to taking this patch (and alternatives if risky): minor String or IDL/UUID changes made by this patch: n/a

(In reply to Mike de Boer [:mikedeboer] from comment #20) > Stephen, I think you're right. The fix should be simple, but will be dealt > with in another bug. Did you file another bug for this? What is it?

Right, I forgot to file it. Will do that right away.