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)
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
Reporter | ||
Updated•15 years ago
|
Version: unspecified → Trunk
Reporter | ||
Updated•15 years ago
|
blocking2.0: --- → ?
Reporter | ||
Updated•15 years ago
|
Summary: Curent Aero implementation is using default Aero borders → Current Aero implementation is using default Aero borders
Updated•15 years ago
|
Component: Theme → Widget: Win32
Product: Firefox → Core
QA Contact: theme → win32
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → tellrob
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 1•15 years ago
|
||
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+
Reporter | ||
Comment 3•15 years ago
|
||
(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).
Reporter | ||
Comment 4•15 years ago
|
||
Assignee | ||
Comment 5•15 years ago
|
||
Hmm. Maybe we need to paint with alpha in the child windows as well.
Comment 6•15 years ago
|
||
Are there other apps not using the border? I'm not convinced removing it improves things.
Reporter | ||
Comment 7•15 years ago
|
||
Tabs looks UGLY with the border, period.
Reporter | ||
Comment 8•15 years ago
|
||
(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.
Comment 9•15 years ago
|
||
The tab strip simply hasn't been updated yet, it's basically still the 3.6 styling.
Comment 10•15 years ago
|
||
Also note that we're using glass not only in the main window. I just landed bug 555221.
Reporter | ||
Updated•15 years ago
|
Summary: Current Aero implementation is using default Aero borders → Current Aero implementation is using default Aero borders in main window
Assignee | ||
Comment 11•15 years ago
|
||
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.
Comment 12•15 years ago
|
||
(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.
Assignee | ||
Comment 13•15 years ago
|
||
(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?
Comment 14•15 years ago
|
||
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".
Reporter | ||
Comment 15•15 years ago
|
||
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.
Updated•15 years ago
|
blocking2.0: ? → final+
Updated•15 years ago
|
Attachment #435088 -
Attachment is obsolete: true
Comment 16•15 years ago
|
||
couldn't you expose a -moz-win-borderless-glass (or whatever you want to name it) and push borders inwards only for this class?
Assignee | ||
Comment 17•15 years ago
|
||
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.
Comment 18•15 years ago
|
||
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.
Comment 19•14 years ago
|
||
Should Bug 560507 be dependent on this?
Reporter | ||
Comment 20•14 years ago
|
||
No, tbas have nothing to do with this. Well, at least not directly.
Comment 22•14 years ago
|
||
(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.
Reporter | ||
Comment 23•14 years ago
|
||
Tabs At Bottom or Tabs On Top?
Reporter | ||
Comment 24•14 years ago
|
||
(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
Comment 25•14 years ago
|
||
(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
Comment 26•14 years ago
|
||
Reporter | ||
Comment 27•14 years ago
|
||
Damn, I really need sleep. I overlooked in which bug I was. Sorry.
Reporter | ||
Comment 28•14 years ago
|
||
Can we land this sooner then for final+?
Assignee | ||
Comment 29•14 years ago
|
||
(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.
Assignee | ||
Comment 30•14 years ago
|
||
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)
Updated•14 years ago
|
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?)
Assignee | ||
Comment 32•14 years ago
|
||
(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.
Attachment #466565 -
Flags: review?(roc) → review+
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.
Assignee | ||
Comment 36•14 years ago
|
||
This was pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/13bfa20e543a
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
Target Milestone: --- → mozilla2.0b5
Assignee | ||
Updated•14 years ago
|
Keywords: dev-doc-needed
Comment 37•14 years ago
|
||
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
Assignee | ||
Comment 38•14 years ago
|
||
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.
Comment 39•14 years ago
|
||
Documented here:
https://developer.mozilla.org/en/CSS/-moz-appearance
Mentioned here:
https://developer.mozilla.org/en/Firefox_4_for_developers
Keywords: dev-doc-needed → dev-doc-complete
Comment 40•14 years ago
|
||
Seems to be linked with new rendering issue with Aero appearance in Vista:
https://bugzilla.mozilla.org/show_bug.cgi?id=589415
You need to log in
before you can comment on or make changes to this bug.
Description
•