Closed Bug 554982 Opened 15 years ago Closed 14 years ago

Current Aero implementation is using default Aero borders in main window

Categories

(Core :: Widget: Win32, defect)

x86
Windows 7
defect
Not set
trivial

Tracking

()

RESOLVED FIXED
mozilla2.0b5
Tracking Status
blocking2.0 --- final+

People

(Reporter: Terepin, Assigned: robarnold)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; sk; rv:1.9.3a4pre) Gecko/20100325 Minefield/3.7a4pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; sk; rv:1.9.3a4pre) Gecko/20100325 Minefield/3.7a4pre According to mockup (https://wiki.mozilla.org/images/3/35/Firefox-4-Mockup-i05-%28Win7%29-%28Aero%29-%28BottomTabs%29.png), there wont be Aero borders (the white line around page content). This should be removed, as the line is interfering with background tabs (line is visible behind them) and it doesn't look very nice (good example is Strata40: http://fc01.deviantart.net/fs70/i/2010/064/e/c/Strata40_v0_6_by_SpewBoy.png). Reproducible: Always
Version: unspecified → Trunk
blocking2.0: --- → ?
Blocks: 546259
Summary: Curent Aero implementation is using default Aero borders → Current Aero implementation is using default Aero borders
Component: Theme → Widget: Win32
Product: Firefox → Core
QA Contact: theme → win32
Assignee: nobody → tellrob
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #435088 - Flags: review?(roc)
Comment on attachment 435088 [details] [diff] [review] If the window is using glass, inset the border by 2px if the opaque rect has dimension <= 4 in width or height, this will effectively set the opaque rect to something with 0 or negative. I guess in that case we just want the whole window to be glass. Please check that works.
Attachment #435088 - Flags: review?(roc) → review+
(In reply to comment #1) > Created an attachment (id=435088) [details] > If the window is using glass, inset the border by 2px There is a bug with this solution (see screenshot).
Attached image white lines insight page content (deleted) —
Hmm. Maybe we need to paint with alpha in the child windows as well.
Are there other apps not using the border? I'm not convinced removing it improves things.
Tabs looks UGLY with the border, period.
(In reply to comment #6) > Are there other apps not using the border? I'm not convinced removing it > improves things. Bot to answer your question, yes, Chrome isn't using it as well.
The tab strip simply hasn't been updated yet, it's basically still the 3.6 styling.
Also note that we're using glass not only in the main window. I just landed bug 555221.
Summary: Current Aero implementation is using default Aero borders → Current Aero implementation is using default Aero borders in main window
I still have not found a way to hide the border without making the entire window glass. Chrome shoves their border under the window by 1px fwiw. I think the fix to do is to push our borders inwards. If the height or width is < 4 pixels, then we make the entire sheet glass and we must draw all child widgets with alpha. (In reply to comment #10) > Also note that we're using glass not only in the main window. I just landed bug > 555221. Can't we just fake the border there with CSS? I'm not sure if/how this should be exposed in CSS. If it can be done in CSS with the existing border options, I'd rather not expose an option in CSS just for this.
(In reply to comment #11) > > Also note that we're using glass not only in the main window. I just landed bug > > 555221. > Can't we just fake the border there with CSS? I don't think we want to.
(In reply to comment #12) > (In reply to comment #11) > > > Also note that we're using glass not only in the main window. I just landed bug > > > 555221. > > Can't we just fake the border there with CSS? > > I don't think we want to. Why not?
Glass without borders would be the exception, so letting every other window jump through hoops doesn't sound right. It seems messy and redundant for something that should be handled natively and "just work".
I think it would be better if this will be fixed before new tabs arrived, otherweise thay will look ugly. Please, keep in mind that Tabs On Top will be affected as well.
blocking2.0: ? → final+
Attachment #435088 - Attachment is obsolete: true
couldn't you expose a -moz-win-borderless-glass (or whatever you want to name it) and push borders inwards only for this class?
I thought about this but I don't like the idea of adding more CSS keywords. Here's an idea I had though (ran it by bz too): the margin property on XUL <window>s is unused. We could use that to set the border offsets. By default they're 0 so we retain the default behavior. For the main window, we'd want to set them to 2px so that we push the border 2px under the opaque area. Either way we have to change nsIWidget. Using margin also means we can support the border on different edges (like everything but the top might look ok for some themes). Probably gets CSS animation support too fwiw.
A separate keyword seems like the cleanest solution from my perspective. I don't quite understand the margin idea; margin is an outer offset, whereas this is about an inner border between the aero and non-aero area.
Should Bug 560507 be dependent on this?
No, tbas have nothing to do with this. Well, at least not directly.
Blocks: 576371
(In reply to comment #20) > No, tbas have nothing to do with this. Well, at least not directly. This bug causes a white line under the tabs though.
Tabs At Bottom or Tabs On Top?
(In reply to comment #23) > Tabs At Bottom or Tabs On Top? Damn, I could wrote the bugs right away. Tabs At Bottom - Bug 554982 Tabs On Top - Bug 571662
(In reply to comment #24) > (In reply to comment #23) > > Tabs At Bottom or Tabs On Top? > > Damn, I could wrote the bugs right away. > Tabs At Bottom - Bug 554982 > Tabs On Top - Bug 571662 What? uh tabs on bottom
Attached image tabs on bottom (deleted) —
Damn, I really need sleep. I overlooked in which bug I was. Sorry.
Can we land this sooner then for final+?
Blocks: 570279
Blocks: 587796
(In reply to comment #18) > A separate keyword seems like the cleanest solution from my perspective. I > don't quite understand the margin idea; margin is an outer offset, whereas this > is about an inner border between the aero and non-aero area. Yeah, I was totally wrong here. Going to make a -moz-win-borderless-glass -moz-appearance and hook it all up.
This patch adds a -moz-win-borderless-glass style which is turned into the new eTransparencyBorderlessGlass transparency mode.
Attachment #466565 - Flags: review?(roc)
Attachment #466565 - Flags: review?(dao)
Attachment #466565 - Flags: review?(dao) → review+
In nsWindow::UpdateGlass, it's OK to pass margins that add up to more than the size of the window? + case eTransparencyGlass: + case eTransparencyBorderlessGlass: + default: + // If we're not doing translucency, then double buffer + doubleBuffering = BasicLayerManager::BUFFER_BUFFERED; + break; + case eTransparencyTransparent: + // If we're rendering with translucency, we're going to be + // rendering the whole window; make sure we clear it first + thebesContext->SetOperator(gfxContext::OPERATOR_CLEAR); + thebesContext->Paint(); + thebesContext->SetOperator(gfxContext::OPERATOR_OVER); + break; So this isn't new, but shouldn't we be clearing the ThebesContext for the glass modes too? (And why did you remove the nsUXThemeData::sHaveCompositor condition?)
(In reply to comment #31) > In nsWindow::UpdateGlass, it's OK to pass margins that add up to more than the > size of the window? Yes, that will make the whole window glass. > + case eTransparencyGlass: > + case eTransparencyBorderlessGlass: > + default: > + // If we're not doing translucency, then double buffer > + doubleBuffering = BasicLayerManager::BUFFER_BUFFERED; > + break; > + case eTransparencyTransparent: > + // If we're rendering with translucency, we're going to be > + // rendering the whole window; make sure we clear it first > + thebesContext->SetOperator(gfxContext::OPERATOR_CLEAR); > + thebesContext->Paint(); > + thebesContext->SetOperator(gfxContext::OPERATOR_OVER); > + break; > > So this isn't new, but shouldn't we be clearing the ThebesContext for the glass > modes too? (And why did you remove the nsUXThemeData::sHaveCompositor > condition?) I don't know why this works but if it didn't I think we would have discovered that by now. We used to PushGroup(ColorAlpha) which gave us a cleared surface to paint on iirc. The change to double buffering w/layers was done here as part of the layers landing: http://hg.mozilla.org/mozilla-central/rev/88a10532e555 I removed the nsUXThemeData::sHaveCompositor condition because it was unecessary - if it was false, we'd end up doing the exact same thing as if it were true.
I think we need a patch to make the glass cases match up the transparent case by clearing the buffer.
That patch doesn't need to be in this bug, though.
Actually, it looks like we don't need this explicit clear for the transparency case.
This was pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/rev/13bfa20e543a
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
Keywords: dev-doc-needed
This isn't fixed yet. It brought back an old regression - there's a white border around the page content. Most notible on dark backgrounds with tabs on bottom. http://img341.imageshack.us/img341/8896/clipboard01x.png
I can confirm on trunk that it happens under some circumstances but thankfully cannot confirm with the patches from bug 130078 applied. Since that is landing next week (if not in the next few days), I do not see much value in filing a bug at this time.
Blocks: 588764
Seems to be linked with new rendering issue with Aero appearance in Vista: https://bugzilla.mozilla.org/show_bug.cgi?id=589415
Depends on: 589471
No longer depends on: 589471
Depends on: 592182
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: