Closed Bug 1185953 Opened 4 years ago Closed 4 years ago

Location bar and search field on Windows 10 should have a 1px border radius

Categories

(Firefox :: Theme, defect, P4)

40 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 42
Tracking Status
firefox40 + fixed
firefox41 --- fixed
firefox42 --- verified

People

(Reporter: phlsa, Assigned: phlsa)

References

Details

Attachments

(1 file, 2 obsolete files)

Priority: -- → P4
Are you sure about this? It's subtle, but seems like other spots in Win10 don't do anything like that.
Yes - I've been running with it as a user style for a while now and it feels better. Corresponds better to our tab shape and slightly rounded icons as well...
It's also been in Stephens spec from the beginning.
(In reply to Philipp Sackl [:phlsa] please use needinfo from comment #2)
> Yes - I've been running with it as a user style for a while now and it feels
> better. Corresponds better to our tab shape and slightly rounded icons as
> well...
> It's also been in Stephens spec from the beginning.

The toolbar buttons don't have any border-radius, so these will be inconsistent. Are you sure you want that?
Flags: needinfo?(philipp)
Comment on attachment 8637898 [details] [diff] [review]
location-bar-border-radius

The right place to set that radius would be here: http://hg.mozilla.org/mozilla-central/annotate/2ddec2dedced/browser/themes/windows/browser.css#l1193

(We'd set it to 2px for XP, Vista and Win 7 a few lines further below.)
Attachment #8637898 - Flags: review?(dolske) → review-
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> (In reply to Philipp Sackl [:phlsa] please use needinfo from comment #2)
> > Yes - I've been running with it as a user style for a while now and it feels
> > better. Corresponds better to our tab shape and slightly rounded icons as
> > well...
> > It's also been in Stephens spec from the beginning.
> 
> The toolbar buttons don't have any border-radius, so these will be
> inconsistent. Are you sure you want that?

The spec for the toolbar buttons actually also calls for the 1px radius there (bug 1173743). I tried to implement that as well, but couldn't figure out how to make the combined bookmark control work. I think we could live with the controls being slightly different for one version, given how much more common interactions with the URL bar and search field are.

Dão, I'll update the patch this afternoon...
Flags: needinfo?(philipp)
Tracking 40 as Philipp flagged this as a bug that we need for Windows 10 support in this release.
Attached patch location-bar-border-radius (obsolete) — Splinter Review
Updated patch
Assignee: nobody → philipp
Attachment #8637898 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8638597 - Flags: review?(dao)
Comment on attachment 8638597 [details] [diff] [review]
location-bar-border-radius

>+@media (-moz-os-version: windows-win10) {
>+  #urlbar,
>+  .searchbar-textbox {
>+    border-radius: 1px;
>+  }
>+}

You should set the border-radius right here:

> @media (-moz-windows-default-theme) {
>   #urlbar,
>   .searchbar-textbox {
>     @navbarTextboxCustomBorder@
------^

FWIW, I agree with dolske and don't really understand the concept behind this. Not having the radius seems to make more sense in the Win10 context, and I don't see this clash with our tab shape and slightly rounded icons. (If you really think this is relevant, have you considered making our icon set for Win10 less rounded?)
Attachment #8638597 - Flags: review?(dao) → review-
(In reply to Dão Gottwald [:dao] from comment #9)
> Comment on attachment 8638597 [details] [diff] [review]
> location-bar-border-radius
> 
> >+@media (-moz-os-version: windows-win10) {
> >+  #urlbar,
> >+  .searchbar-textbox {
> >+    border-radius: 1px;
> >+  }
> >+}
> 
> You should set the border-radius right here:
> 
> > @media (-moz-windows-default-theme) {
> >   #urlbar,
> >   .searchbar-textbox {
> >     @navbarTextboxCustomBorder@
> ------^

Hm, this also changes the behavior on Windows 8, which I think we don't want.

> FWIW, I agree with dolske and don't really understand the concept behind
> this. Not having the radius seems to make more sense in the Win10 context,
> and I don't see this clash with our tab shape and slightly rounded icons.
> (If you really think this is relevant, have you considered making our icon
> set for Win10 less rounded?)

Stephen once experimented with different icons, but I think the conclusion was that they weren't Firefoxy enough. Generally, we're in a space where we are balancing platform identity and product identity. Sometimes that happens in obvious ways (round tabs), sometimes in less obvious ones (the corners). It's always hard to make clear-cut decisions about where small details like this put us on that range, which is why I am trusting Stephens (and to a lesser extent my own) gut on this one.
(In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from comment #10)
> (In reply to Dão Gottwald [:dao] from comment #9)
> > Comment on attachment 8638597 [details] [diff] [review]
> > location-bar-border-radius
> > 
> > >+@media (-moz-os-version: windows-win10) {
> > >+  #urlbar,
> > >+  .searchbar-textbox {
> > >+    border-radius: 1px;
> > >+  }
> > >+}
> > 
> > You should set the border-radius right here:
> > 
> > > @media (-moz-windows-default-theme) {
> > >   #urlbar,
> > >   .searchbar-textbox {
> > >     @navbarTextboxCustomBorder@
> > ------^
> 
> Hm, this also changes the behavior on Windows 8, which I think we don't want.

Why not? I haven't used Windows 8 and 10 a lot, but I got the impression that 10 is more consistently flat and rectangular than 8. So in terms of platform integration this would make more sense on 8 than on 10. And what you said about product identity is no less true on 8, is it?
(In reply to Dão Gottwald [:dao] from comment #11)
> (In reply to Philipp Sackl [:phlsa] (Firefox UX) please use needinfo from
> comment #10)
> > (In reply to Dão Gottwald [:dao] from comment #9)
> > > Comment on attachment 8638597 [details] [diff] [review]
> > > location-bar-border-radius
> > > 
> > > >+@media (-moz-os-version: windows-win10) {
> > > >+  #urlbar,
> > > >+  .searchbar-textbox {
> > > >+    border-radius: 1px;
> > > >+  }
> > > >+}
> > > 
> > > You should set the border-radius right here:
> > > 
> > > > @media (-moz-windows-default-theme) {
> > > >   #urlbar,
> > > >   .searchbar-textbox {
> > > >     @navbarTextboxCustomBorder@
> > > ------^
> > 
> > Hm, this also changes the behavior on Windows 8, which I think we don't want.
> 
> Why not? I haven't used Windows 8 and 10 a lot, but I got the impression
> that 10 is more consistently flat and rectangular than 8. So in terms of
> platform integration this would make more sense on 8 than on 10. And what
> you said about product identity is no less true on 8, is it?

I'm not even disputing that, but for Windows 10 we have a design spec where these round corners have been considered, weighted against other options, and deemed the best choice. For Windows 8 it would be a side effect. I'm happy to consider that, but I don't want to do it when we are in a rush while we are racing to finish a product on a different platform.
Just tested this on various scale factors on Windows 8. Seems legit...
Attachment #8638597 - Attachment is obsolete: true
Attachment #8639222 - Flags: review?(dao)
Attachment #8639222 - Flags: review?(dao) → review+
Comment on attachment 8639222 [details] [diff] [review]
location-bar-border-radius-2

Approval Request Comment
[Feature/regressing bug #]: Windows 10
[User impact if declined]: Worse-looking navigation and search bar
[Describe test coverage new/current, TreeHerder]: Local testing
[Risks and why]: Low, 1-line CSS change
[String/UUID change made/needed]: None
Attachment #8639222 - Flags: approval-mozilla-beta?
Attachment #8639222 - Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
This will get resolved when the patch has been merged to mozilla-central.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/35ac9b7fbbe3
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Comment on attachment 8639222 [details] [diff] [review]
location-bar-border-radius-2

Trivial change in support of Windows 10. Beta+ Aurora+
Attachment #8639222 - Flags: approval-mozilla-beta?
Attachment #8639222 - Flags: approval-mozilla-beta+
Attachment #8639222 - Flags: approval-mozilla-aurora?
Attachment #8639222 - Flags: approval-mozilla-aurora+
Reproduce this bug on Firefox Nightly 42.0a1 (2015-07-21) on windows 10 64 bit

The bug's fix is verified on latest Firefox Beta 42.0b3 (Build ID:20151001142456)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:42.0) Gecko/20100101 Firefox/42.0
QA Whiteboard: [testday-20151002]
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.