980418 - about:preferences squaring

Status: RESOLVED WONTFIX

(Toolkit :: Themes, defect)

Description

[Peter Henkel [:Terepin]]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140306030201

Steps to reproduce:

Options list and section selector needs to be squared and not rounded.

Comment 1

[Tim Nguyen :ntim]

Attachment: bug_980418.patch

Main content squaring is bug 980339.
This patch removes border-radius for Windows 8.

Comment 2

[Tim Nguyen :ntim]

Attachment: Patch v2

Comment 3

[Mike de Boer [:mikedeboer]]

Comment on attachment 8397295 [details] [diff] [review]
Patch v2

Review of attachment 8397295 [details] [diff] [review]:
-----------------------------------------------------------------

If you make the change as I commented below, you'll have an r+!

::: browser/themes/windows/preferences/in-content/preferences.css
@@ +49,5 @@
>  }
>  
> +%ifdef WINDOWS_AERO
> +@media (-moz-os-version: windows-vista),
> +       (-moz-os-version: windows-win7) {

Sorry, I think I gave you the wrong info :/

This won't work for Windows XP, for example. So, instead, this needs to be

```css
%ifdef WINDOWS_AERO
@media (-moz-os-version: windows-vista),
       (-moz-os-version: windows-win7) {
%endif
  .category:-moz-locale-dir(ltr) {
    border-top-left-radius: 5px;
    border-bottom-left-radius: 5px;
  }
		 
  .category:-moz-locale-dir(rtl) {
    border-top-right-radius: 5px;
    border-bottom-right-radius: 5px;
  }
%ifdef WINDOWS_AERO
}
%endif
```

Media queries and our Windows theme... you never stop learning!

Comment 4

[u428464]

Since bug 738796 will normally land soon. I don't think this is necessary to land this.

Comment 5

[Mike de Boer [:mikedeboer]]

(In reply to Guillaume C. [:ge3k0s] from comment #4)
> Since bug 738796 will normally land soon. I don't think this is necessary to
> land this.

Ah, true! Thanks for pointing this out, Guillaume. However, what about Aurora and Beta? I'm pretty sure that the revamped in-content prefs won't get uplifted, so we still might want this for the other channels.

Comment 6

[u428464]

(In reply to Mike de Boer [:mikedeboer] from comment #5)
> (In reply to Guillaume C. [:ge3k0s] from comment #4)
> > Since bug 738796 will normally land soon. I don't think this is necessary to
> > land this.
> 
> Ah, true! Thanks for pointing this out, Guillaume. However, what about
> Aurora and Beta? I'm pretty sure that the revamped in-content prefs won't
> get uplifted, so we still might want this for the other channels.

Yes the in-content preferences won't be enabled right now even on Nightly, but about:preferences is hidden for now (the window is still enabled by default) and only used by a tiny minority of users (some of it is outdated, e.g. Fxa), so I'm not sure it's worth the effort to change it.

Comment 7

[Tim Nguyen :ntim]

Attachment: Patch v3

Comment 8

[Peter Henkel [:Terepin]]

(In reply to Guillaume C. [:ge3k0s] from comment #4)
> Since bug 738796 will normally land soon. I don't think this is necessary to
> land this.

It landed and now options ICUI looks like as if someone slapped facehuger on dead body with decapitated head. Either that, or I have serious perceiving issues.

I filled this bug too soon (or too late, depending on how you are looking at it). Since arguably this patch doesn't make sense anymore, I'm therefor proposing to close it as duplicate of proper Windows 8 design.

Comment 9

[Mike de Boer [:mikedeboer]]

(In reply to Peter Henkel [:Terepin] from comment #8)
> It landed and now options ICUI looks like as if someone slapped facehuger on
> dead body with decapitated head. Either that, or I have serious perceiving
> issues.

Well, I must say you honor your avatar with this kind of grumpiness. Please consider others' feelings and respect their hard work, before you criticize with gruesome metaphors like this!

> I filled this bug too soon (or too late, depending on how you are looking at
> it). Since arguably this patch doesn't make sense anymore, I'm therefor
> proposing to close it as duplicate of proper Windows 8 design.

I agree, sort of. The new style in-content preferences UI is not present on Fx 30 and 29, so the patch would still make sense there. I do, however, think we should leave things as-is, but more due to reasons mentioned by Guillaume in comment 6.

It's too costly to accept the patch for a slight visual improvement for a piece of UI that only a small number of users see (no telemetry-style hard facts here, just a common-sense assertion) wrt branch-landing, uplift, tracking, QA cycles, etc.

Tim, I appreciate your work done here, but I think you'll agree with me that your time & effort will also be better spent on the other bugs you're working on!

Thanks!

Comment 10

[Peter Henkel [:Terepin]]

Are there bugs for the rest OS' designs?

Comment 11

[Mike de Boer [:mikedeboer]]

(In reply to Peter Henkel [:Terepin] from comment #10)
> Are there bugs for the rest OS' designs?

What do you mean with OS?

Comment 12

[Peter Henkel [:Terepin]]

Operating Systems. New ICUI design shown in about:preferences was made for Mac OSX only, despite provided info in bug 738796.

Comment 13

[Mike de Boer [:mikedeboer]]

(In reply to Peter Henkel [:Terepin] from comment #12)
> Operating Systems. New ICUI design shown in about:preferences was made for
> Mac OSX only, despite provided info in bug 738796.

Peter, the design is meant to look the same on all OSes, so there won't be any changes made for any OS specifically. For the things you think don't look right about the ICUI preferences at this point, please feel free to file bugs that block bug 738796!

Also, I'd like to suggest to move this discussion to that bug or the firefox-dev mailing list, if you want to host a more general discussion.