988536 - InContent preferences needs icons for headers and arrows.

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Richard Marti (:Paenglab)]

Bug 738796 is now using some provisional icons.

Icons needed:
- Header icons. 40px and HiDPI
- Arrow icons for the dropdowns. LoDPI and HiDPI
- Arrow icons for Application pane treecol header. LoDPI and HiDPI

Comment 2

[Michael Maslaney [:mmaslaney] ([e-mail])]

Attachment: inContent_icons.zip

Let me know if you need additional assets.

Comment 3

[Richard Marti (:Paenglab)]

Thank you for the files. Missing are Content_Icon.png and [e-mail] for the header.

Comment 4

[Michael Maslaney [:mmaslaney] ([e-mail])]

Attachment: Content_icon.zip

Comment 5

[Richard Marti (:Paenglab)]

I reopen this bug for the needed patch I'll attach later.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: inContentIcons.patch

This patch adds the needed icons. It also moves all icons to shared because they are for all themes the same.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8401109 [details] [diff] [review]
inContentIcons.patch

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

Looks good!

::: browser/themes/linux/jar.mn
@@ +134,5 @@
>    skin/classic/browser/preferences/Options-sync.png   (preferences/Options-sync.png)
>  #endif
>  * skin/classic/browser/preferences/preferences.css    (preferences/preferences.css)
>  * skin/classic/browser/preferences/in-content/preferences.css (preferences/in-content/preferences.css)
> +  skin/classic/browser/preferences/in-content/check.png       (../shared/in-content/check.png)

I'd prefer if the directory under 'shared' would be called 'incontentprefs' instead of just 'in-content'. I removed the hyphens since customizableui doesn't have a hyphen.

::: browser/themes/osx/preferences/in-content/preferences.css
@@ +158,5 @@
>    }
> +
> +  .header-icon {
> +    list-style-image: url("chrome://browser/skin/preferences/in-content/[e-mail]");
> +  }

Can you move these @2x styles in to the shared CSS file? They're duplicated between OSX and Windows, and I don't see why we need to keep separate copies of them.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: inContentIcons.patch

Using now shared/incontentprefs instead of shared/in-content and moved the rules for HiDPI to the shared preferences.css.

I also added the @2x images to Linux to avoid rules pointing to nonexistent files.

Comment 9

[u428464]

Would be nice to have the icons less blurry. The patch seems to be ready, just waiting on review. :)

Comment 10

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8401391 [details] [diff] [review]
inContentIcons.patch

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

Looks good! Thanks!

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/1058d4010f32

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1058d4010f32

Comment 13

[Camelia Badau [:cbadau], Desktop Test Engineering]

Verified fixed on Windows 7 64bit, Ubuntu 13.10 and Mac OSX 10.7.5 using Nightly 31.0a1 (buildID: 20140428030203).