9.32 KB, image/png
69.69 KB, image/png
11.81 KB, image/png
1.78 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b11pre) Gecko/20110128 Firefox/4.0b11pre Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b11pre) Gecko/20110128 Firefox/4.0b11pre The latest nightly (2011-01-28) introduced an unwanted white line above the navbar. It's only present in maximized window. See the screenshot. Reproducible: Always
Component: Theme → General
Product: Firefox → Core
QA Contact: theme → general
This probably won't be hard to track down, I'm guessing it's the Windows glass border leaking through after our changes to how we calculate the glass margins. I'd suggest this block as a soft blocker. If it opens a can of worms we probably don't want to worry about it, but if there's an easy fix, we should take it.
blocking2.0: --- → ?
Interesting, the titlebar on win7 is 32px, and our margin gets set to 32px. But we've moved the client border out to zero via chrome margins, so I don't understand (if or why) windows would be drawing anything along the glass edge. Maybe this is something else.
Ok I think I got it. This is how Windows renders the edge between the glass/opaque area. It's a 1px white border + 1px darker border (just like the window edges). Those are the white border mentioned here and the reason for the darker borders in bug 629722. The kGlassMarginsAdjustment was meant to be a 2px increase in the final margin calculated, so that these borders fall completely behind the opaque area. But if plugins intersecting with that border look incorrect, then it's one bug against another
Summary: White line appears above navtoolbar if the window is maximized → White line of highlight pixels appears above navigation toolbar if the window is maximized
Created attachment 508091 [details] white line in addons manager the same white line appears when the addons manager is open also in non maximized window
Ok, I see what's going on here. I thought windows would only paint this border at the client edge, but it paints it at the glass edge. Working on a fix.
Assignee: nobody → jmathies
Just to add more info, on non-maximized mode this line is not visible because the top of the toolbar has rounded corners, which excludes the toolbar from the opaque region
Created attachment 508162 [details] border It's unfortunate but we have to make a trade off. The margin calculation changes in bug 622328 improved on the white line that borders plugins, but introduced bug 629722 because the glass was set to 2px instead of 3px. This bug was just a side effect of a bad assumption on my part. So I think the fix here is to just back out the margins changes (results attached in an image) and live with the ugly anomaly with windowed plugins.
Comment on attachment 508168 [details] [diff] [review] undo margin changes but keep logging statement Alternatively I could back out the cset that had this a reland the log statement. Either way.
Attachment #508168 - Flags: review?(roc)
Attachment #508168 - Flags: review?(roc) → review+
Don't think this blocks, but let's keep on the path and get it in through approvals.
blocking2.0: ? → -
(In reply to comment #11) > Don't think this blocks, but let's keep on the path and get it in through > approvals. This is just backing out some changes in bug 622328 that introduced a regression, so it should probably land for 4.0.
Comment on attachment 508168 [details] [diff] [review] undo margin changes but keep logging statement Asking for approval for this partial backout to fix a really ugly visual regression (on mac the equivalent would be regressing the unified toolbar, IMO). The risk seems small and the benefits are large.
Attachment #508168 - Flags: approval2.0?
Is there any way for us to test the behavior we want here so we don't accidentally regress it in the future? (not sure if we can reftest a whole window)
(In reply to comment #14) > Is there any way for us to test the behavior we want here so we don't > accidentally regress it in the future? (not sure if we can reftest a whole > window) Ref testing glass is hard since the background shows through. But the chance of regressing this specific glass margins problem is pretty slim. Roc and I have both worked on this code, so it's pretty well set what we do in this routine. Changes here aren't very likely.
Attachment #508168 - Flags: approval2.0? → approval2.0+
When probably will land this?
Component: General → Widget: Win32
QA Contact: general → win32
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
I backed this patch as part of this pushlog <http://hg.mozilla.org/mozilla-central/pushloghtml?changeset=7e12e3e16e6c> because of the oranges it caused <http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1296852850.1296854923.2345.gz&fulltext=1>. I'm not sure which one of the bugs was at fault, so I just backed them all out. The assignee needs to investigate and make sure that his patch has not been the culprit, and then re-add checkin-needed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Relanded all but one of the patches in that push. For this bug, that's: http://hg.mozilla.org/mozilla-central/rev/d49d590321e9
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago → 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.