Closed Bug 1125657 Opened 9 years ago Closed 9 years ago

Window borders should be 0px thick in Windows 10

Categories

(Firefox :: Theme, defect, P2)

All
Windows 10
defect

Tracking

()

VERIFIED FIXED
Firefox 41
Tracking Status
firefox39 --- wontfix
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: josh.tumath+bugzilla, Assigned: Gijs)

References

Details

(Whiteboard: [testday-20150901])

Attachments

(2 files)

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.
Blocks: windows-10
Component: Untriaged → Theme
OS: Windows NT → Windows 10
Hardware: x86 → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 1097604
Priority: -- → P2
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.
(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
Bug 1125657 - remove grey borders on windows 10, r?dao
Attachment #8613513 - Flags: review?(dao)
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-
(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)
(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)
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)
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.
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?
(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.
Yeah, please just move the rule. background-clip is only needed because @toolbarShadowColor@ has transparency. Without that, background-clip won't be needed either.
Attachment #8613513 - Flags: review?(dao)
Attachment #8613513 - Flags: review?(dao)
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?(dao) → review+
https://hg.mozilla.org/mozilla-central/rev/4017f25a8733
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
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?
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+
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.