Closed Bug 462771 Opened 14 years ago Closed 14 years ago

Avoid use of transparent PNGs in chrome

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Maemo
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
fennec1.0a2

People

(Reporter: pavlov, Assigned: Gavin)

References

Details

Attachments

(1 file, 12 obsolete files)

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.
Attached patch WIP (obsolete) — Splinter Review
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.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
One more problem:
-the autocomplete popup's background is still partially transparent
(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).
-navigation_magnifier background does not match navigation_search_caps background gradient
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.
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.
Attached image UPDATED: close_small.png (obsolete) —
close_small.png updated with uncompensated color (no profile) and background set to the proper color.
(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.
Attached image UPDATED: addressbar_endcap_l_on.png (obsolete) —
Attached image UPDATED: addressbar_endcap_l_off.png (obsolete) —
Attached image UPDATED: addressbar_endcap_r_on.png (obsolete) —
Attached image UPDATED: addressbar_endcap_r_off.png (obsolete) —
Attached patch updated patch (obsolete) — Splinter Review
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
Attachment #347130 - Attachment is obsolete: true
Attachment #347208 - Attachment is obsolete: true
Attachment #347209 - Attachment is obsolete: true
Attachment #347210 - Attachment is obsolete: true
Attachment #347211 - Attachment is obsolete: true
Attachment #347212 - Attachment is obsolete: true
Attachment #347213 - Attachment is obsolete: true
Attachment #347216 - Attachment is obsolete: true
Attached image UPDATED: panel_background.png (obsolete) —
aligned gradients with toolbar-background
Attached image UPDATED: toolbar-background.png (obsolete) —
centered the gradients within the top strip so the light/dark are of equal size allowing elements to be vertically centered.
Attached image UPDATED: starred48.png (obsolete) —
updated background color to rgb 207,207,207.
Attached patch patchSplinter Review
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.
Attachment #347227 - Attachment is obsolete: true
Attachment #347436 - Attachment is obsolete: true
Attachment #347437 - Attachment is obsolete: true
Attachment #347438 - Attachment is obsolete: true
Attachment #347642 - Flags: review?(mark.finkle)
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;
Attachment #347642 - Flags: review?(mark.finkle) → review+
http://hg.mozilla.org/mobile-browser/rev/7f7fb29440d0
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Fennec A2
Depends on: 464491
Depends on: 464490
Depends on: 464492
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.