462771 - Avoid use of transparent PNGs in chrome

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Stuart Parmenter]

Any time we have to composite 8bit alpha it is going to slow us down.  Given we know what our UI should look like, we should make sure to have all our icons and backgrounds are opaque.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: WIP

This is complete and includes the new images that Sean sent me this weekend. Some minor problems remain:

-panel_background is tiled horizontally along the "extras" sidebar, so the backgrounds in panel_buttons don't match for the top buttons (addons and downloads)
-close-small is only used in the lighter themed panels at the moment, so needs a lighter background for now: rgb(133, 135, 133)
-need addressbar_endcap images updated

This also fixes the alignment of the side bar buttons to make them match up with the bookmarks button on the URL bar.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

One more problem:
-the autocomplete popup's background is still partially transparent

Comment 3

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #2)
> One more problem:
> -the autocomplete popup's background is still partially transparent

I think this is because navigation_background.png still has transparency (I didn't get a new version from Sean).

Comment 4

[:Gavin Sharp [email: [e-mail]]]

-navigation_magnifier background does not match navigation_search_caps background gradient

Comment 5

[Sean Martell]

For the panel_buttons issue, the original design called for the buttons to be on the solid color area aligned bottom as in (http://blog.seanmartell.com/wp-content/uploads/2008/09/fennec_screen_53.jpg)

Could we try and align the icons as in that mockup, to associate them with the selected prefs button in the main panel, or were they aligned top for another reason?

If we were to do that, we wouldn't have to match a gradient behind the buttons.

Comment 6

[Sean Martell]

Attachment: UPDATED: opaque navigation_background.png

This PNG is now simply a flat color without transparency set to rgb 89,89,89 throughout, if you would rather set the background within the code.

Comment 7

[Sean Martell]

Attachment: UPDATED: close_small.png

close_small.png updated with uncompensated color (no profile) and background set to the proper color.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #5)
> Could we try and align the icons as in that mockup, to associate them with the
> selected prefs button in the main panel, or were they aligned top for another
> reason?

Ah, sure. I probably just forgot to do that, so they defaulted to the top. I can align them on the bottom.

Comment 9

[Sean Martell]

Attachment: UPDATED: addressbar_endcap_l_on.png

Comment 10

[Sean Martell]

Attachment: UPDATED: addressbar_endcap_l_off.png

Comment 11

[Sean Martell]

Attachment: UPDATED: addressbar_endcap_r_on.png

Comment 12

[Sean Martell]

Attachment: UPDATED: addressbar_endcap_r_off.png

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Attachment: screenshot of the search button

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Attachment: updated patch

OK, this is the latest updates, with all of the new images Sean sent me. It addressed all of the problems with the previous patch, but still has a few issues:

-new titlebar image is good, but we don't need the disabled state for bookmarks button (I cropped it locally, just an FYI for future image updates)
-the new navigation_magnifier still doesn't look right. this might be my fault for not stripping the color profiles the first time around. http://people.mozilla.com/~gavin/fennec/search_button2.png
-"light" portion of toolbar-background gradient is 3 px smaller than the "dark" portion, so the addressbar_endcap images can't be aligned correctly within toolbar-background. see http://people.mozilla.com/~gavin/fennec/titlebar.png
-I was mistaken, close-small is actually used on two different backgrounds. Probably easiest to just leave it transparent for now
-panel_background top gradient looks kind of weird - see http://people.mozilla.com/~gavin/fennec/panel.png
-starred48.png background needs to be rgb(207,207,207) for now since it's used in a lighter panel

Comment 15

[Sean Martell]

Attachment: UPDATED: panel_background.png

aligned gradients with toolbar-background

Comment 16

[Sean Martell]

Attachment: UPDATED: toolbar-background.png

centered the gradients within the top strip so the light/dark are of equal size allowing elements to be vertically centered.

Comment 17

[Sean Martell]

Attachment: UPDATED: starred48.png

updated background color to rgb 207,207,207.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Fixes all of the previous issues, apart from those mentioned below. There's a fair bit of "random cleanup" that I just couldn't stop myself from doing, which makes this hard to actually review, but I think it leaves the theme in a better state (better organized, more comments about why certain values are used, etc.).

I've renamed a couple of the images for consistency:
toolbar-background -> toolbar_background
titlebar.png -> toolbar.png

Remaining issues:
-close-small is actually used on two different backgrounds
-page-starred background doesn't match middle part of navigation_url_caps and doesn't fit in when selected
	http://people.mozilla.com/~gavin/fennec/navigation.png

For these two, I think the solution is to just make them transparent.

-top gradient of toolbar_background still does not align with top gradient of new panel_background
	http://people.mozilla.com/~gavin/fennec/panel3.png
-navigation_magnifier background still doesn't match (this might be my fault for not stripping the color profiles the first time around)
	http://people.mozilla.com/~gavin/fennec/search_button2.png

I'll file bugs on these.

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 347642 [details] [diff] [review]
patch


>-#toolbar-main {
>-  -moz-appearance: none;
>-  -moz-box-align: center;
>-  background-image: url("chrome://browser/skin/images/toolbar-background.png");
>-  background-repeat: repeat-x;
>-  background-position: top left;
>-  padding: 4px 12px;
>-  border: none;
>+/* main toolbar buttons */
>+toolbarbutton.urlbar-button {
>+  list-style-image: url("chrome://browser/skin/images/toolbar.png");
>+  /*  */

What was this comment supposed to be?

>+toolbarbutton.panel-button {
>+  list-style-image: url("chrome://browser/skin/images/panel_buttons.png");
>+  background: none !important;            /* override checked style */
>+  border-color: transparent !important;   /* override checked style */
>+}

Switch to your new style for comments?

  /* override checked style */
  background: none !important;
  border-color: transparent !important;

Comment 20

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/mobile-browser/rev/7f7fb29440d0

Comment 21

[Aakash Desai [:aakashd]]

verified FIXED on builds:


Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20090821
Fennec/1.0b3pre

and

Mozilla/5.0 (Macintosh; U; Intel Mac OSX 10.5; en-US; rv:1.9.2a2pre)
Gecko/20090808 Fennec/1.0b3pre