Closed
Bug 1125657
Opened 10 years ago
Closed 9 years ago
Window borders should be 0px thick in Windows 10
Categories
(Firefox :: Theme, defect, P2)
Tracking
()
VERIFIED
FIXED
Firefox 41
People
(Reporter: josh.tumath+bugzilla, Assigned: Gijs)
References
Details
(Whiteboard: [testday-20150901])
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
text/x-review-board-request
|
dao
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta-
|
Details |
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150125030202
Actual results:
The Australis has a grey 1px border around the right, bottom and left sides of the window chrome and content.
Expected results:
Windows 10 has zero distance between window frame content and the edge. The border should probably be removed to fit in with other apps.
Reporter | ||
Updated•10 years ago
|
Updated•10 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•10 years ago
|
No longer blocks: windows-10
Updated•10 years ago
|
Blocks: theme-win10
Updated•10 years ago
|
Priority: -- → P2
Comment 1•9 years ago
|
||
It looks like they are switching to a 1 pixel window border (for active windows at least) that uses the system highlight color. Although this seems to change every build.
Assignee | ||
Comment 2•9 years ago
|
||
(In reply to Stephen Horlander [:shorlander] from comment #1)
> Created attachment 8612289 [details]
> System Highlight Color Uses
>
> It looks like they are switching to a 1 pixel window border (for active
> windows at least) that uses the system highlight color. Although this seems
> to change every build.
Yes, but we have our own grey border which we should probably get rid of, assuming this change sticks around. This is a separate issue from our other colored border issues at the top of the window.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•9 years ago
|
||
Bug 1125657 - remove grey borders on windows 10, r?dao
Attachment #8613513 -
Flags: review?(dao)
Comment 4•9 years ago
|
||
Comment on attachment 8613513 [details]
MozReview Request: Bug 1125657 - remove grey borders on windows 10, r?dao
I tried to use reviewboard but don't find it intuitively usable without spending more time on it than I'm willing to spend right now, so I'll just comment here.
/* Hide the borders on windows 10: */
@media not all and (-moz-os-version: windows-win10) {
Please make this target vista, 7 and 8 such that this becomes forward-compatible, i.e. covers Windows 10 and later rather than just Windows 10. Of course, we don't know what future version of Windows will look like, but it's likely that it will be closer to Windows 10 than to older versions.
@media (-moz-os-version: windows-win10) {
#main-window[sizemode=normal] #browser-bottombox {
/* The top is already set to none in compositor mode in the styles above, and we
* want to get rid of the left, right and bottom ones: */
border-style: none;
}
}
Looks like this might interfere with this rule:
http://hg.mozilla.org/mozilla-central/annotate/56241c1f8a3b/browser/themes/windows/browser.css#l2703
Instead, can you exclude Windows 10 and later where we set the left, right and bottom borders in the first place?
Also, shouldn't we stop using -moz-win-borderless-glass so we get the native borders from attachment 8612289 [details]?
Attachment #8613513 -
Flags: review?(dao) → review-
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #4)
> Comment on attachment 8613513 [details]
> MozReview Request: Bug 1125657 - remove grey borders on windows 10, r?dao
>
> I tried to use reviewboard but don't find it intuitively usable without
> spending more time on it than I'm willing to spend right now, so I'll just
> comment here.
>
> /* Hide the borders on windows 10: */
> @media not all and (-moz-os-version: windows-win10) {
>
> Please make this target vista, 7 and 8 such that this becomes
> forward-compatible, i.e. covers Windows 10 and later rather than just
> Windows 10. Of course, we don't know what future version of Windows will
> look like, but it's likely that it will be closer to Windows 10 than to
> older versions.
Windows 10 is the "last version of Windows", MS has said publicly ( http://www.theverge.com/2015/5/7/8568473/windows-10-last-version-of-windows has some detail). I don't think it will be useful to use the more complicated list-of-versions strategy here. Do you disagree?
> @media (-moz-os-version: windows-win10) {
> #main-window[sizemode=normal] #browser-bottombox {
> /* The top is already set to none in compositor mode in the styles
> above, and we
> * want to get rid of the left, right and bottom ones: */
> border-style: none;
> }
> }
>
> Looks like this might interfere with this rule:
> http://hg.mozilla.org/mozilla-central/annotate/56241c1f8a3b/browser/themes/
> windows/browser.css#l2703
>
> Instead, can you exclude Windows 10 and later where we set the left, right
> and bottom borders in the first place?
I will investigate this.
> Also, shouldn't we stop using -moz-win-borderless-glass so we get the native
> borders from attachment 8612289 [details]?
We already get those borders, but yes, it's probably right to stop using that because of some of the other bugs (e.g. bug 1169911, bug 1167218, bug 1167257). I'd like to do that in one of those bugs, though, if that's OK?
Flags: needinfo?(dao)
Comment 6•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to Dão Gottwald [:dao] from comment #4)
> > Comment on attachment 8613513 [details]
> > MozReview Request: Bug 1125657 - remove grey borders on windows 10, r?dao
> >
> > I tried to use reviewboard but don't find it intuitively usable without
> > spending more time on it than I'm willing to spend right now, so I'll just
> > comment here.
> >
> > /* Hide the borders on windows 10: */
> > @media not all and (-moz-os-version: windows-win10) {
> >
> > Please make this target vista, 7 and 8 such that this becomes
> > forward-compatible, i.e. covers Windows 10 and later rather than just
> > Windows 10. Of course, we don't know what future version of Windows will
> > look like, but it's likely that it will be closer to Windows 10 than to
> > older versions.
>
> Windows 10 is the "last version of Windows", MS has said publicly (
> http://www.theverge.com/2015/5/7/8568473/windows-10-last-version-of-windows
> has some detail). I don't think it will be useful to use the more
> complicated list-of-versions strategy here. Do you disagree?
That's mostly a question of how Windows is being delivered to users. Doesn't mean the theme won't change, and who knows they'll never bump the version number; I don't want to count on that.
> > Also, shouldn't we stop using -moz-win-borderless-glass so we get the native
> > borders from attachment 8612289 [details]?
>
> We already get those borders, but yes, it's probably right to stop using
> that because of some of the other bugs (e.g. bug 1169911, bug 1167218, bug
> 1167257). I'd like to do that in one of those bugs, though, if that's OK?
ok
Flags: needinfo?(dao)
Assignee | ||
Comment 7•9 years ago
|
||
Comment on attachment 8613513 [details]
MozReview Request: Bug 1125657 - remove grey borders on windows 10, r?dao
Bug 1125657 - remove grey borders on windows 10, r?dao
Attachment #8613513 -
Flags: review- → review?(dao)
Assignee | ||
Comment 8•9 years ago
|
||
Super-short reviewboard explanation, you can see the diff here:
https://reviewboard.mozilla.org/r/9755/diff/2/#index_header
clicking a line number, or click-dragging over a set of line numbers lets you make comments regarding that line / those lines.
clicking "Ship it" (or maybe "fix it, then ship it" if you make comments before) will let you set r+ and post the review comments both here and on reviewboard. You might need to be logged in with the relevant bugzilla / ldap account, I don't really remember how the auth story works there.
Comment 9•9 years ago
|
||
What's the point of border-top: 1px none @toolbarShadowColor@;? Why is background-clip: padding-box; not in the rule setting the left, right, and bottom border? Why did you change that rule at all?
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #9)
> What's the point of border-top: 1px none @toolbarShadowColor@;?
I simply cared about leaving as much the same as possible. I don't know if a more specific rule later / elsewhere enables that border by just changing border-top-style. This seemed safer. I could leave it out if you prefer; it might break something, or it might not. I didn't look for uses elsewhere at the time.
> Why is
> background-clip: padding-box; not in the rule setting the left, right, and
> bottom border? Why did you change that rule at all?
Again, this seemed safer - I don't know if/when we ever enable that top border, but this seemed like it changed as little as possible. We certainly do change the background, and without the background-clip, the background would show underneath not-fully-opaque borders on windows 10.
Looking through our CSS, I don't *see* any cases where we do enable the top border (which, tbh, I find a little surprising), and the only case where we enable other borders is the one in comment #4, which has background-clip already. I'm still concerned I missed something, but sure, I can change the patch to just move the rule wholesale.
Comment 11•9 years ago
|
||
Yeah, please just move the rule. background-clip is only needed because @toolbarShadowColor@ has transparency. Without that, background-clip won't be needed either.
Updated•9 years ago
|
Attachment #8613513 -
Flags: review?(dao)
Assignee | ||
Updated•9 years ago
|
Attachment #8613513 -
Flags: review?(dao)
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8613513 [details]
MozReview Request: Bug 1125657 - remove grey borders on windows 10, r?dao
Bug 1125657 - remove grey borders on windows 10, r?dao
Updated•9 years ago
|
Attachment #8613513 -
Flags: review?(dao) → review+
Comment 14•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8613513 [details]
MozReview Request: Bug 1125657 - remove grey borders on windows 10, r?dao
Approval Request Comment
[Feature/regressing bug #]: windows 10
[User impact if declined]: grey borders around the content area which look out of place
[Describe test coverage new/current, TreeHerder]: none, css change only
[Risks and why]: none, media-query-based change only for win10
[String/UUID change made/needed]: no
Attachment #8613513 -
Flags: approval-mozilla-beta?
Attachment #8613513 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
status-firefox39:
--- → wontfix
status-firefox40:
--- → affected
Comment 16•9 years ago
|
||
Comment on attachment 8613513 [details]
MozReview Request: Bug 1125657 - remove grey borders on windows 10, r?dao
Windows 10 changes are planned for fx 40.
Attachment #8613513 -
Flags: approval-mozilla-beta?
Attachment #8613513 -
Flags: approval-mozilla-beta-
Attachment #8613513 -
Flags: approval-mozilla-aurora?
Attachment #8613513 -
Flags: approval-mozilla-aurora+
Comment 17•9 years ago
|
||
Comment 18•9 years ago
|
||
This has been verified fixed in a QA test day.
Status: RESOLVED → VERIFIED
Whiteboard: [testday-20150901]
You need to log in
before you can comment on or make changes to this bug.
Description
•