606590 - style backgrounds of add-ons section dividers

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Madhava Enros [:madhava]]

Currently the are flat grey, as seen here:
http://www.flickr.com/photos/upload/edit/?ids=5106010030

Can we use the same gradient as on the prefs screen?  Or maybe we need a new one?

Comment 1

[Wesley Johnston (:wesj)]

Attachment: Patch

Moves the same style over

Comment 2

[Wesley Johnston (:wesj)]

Attachment: Screenshot

Comment 3

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

Comment on attachment 485458 [details] [diff] [review]
Patch

Remove the extra "color: black" before landing.

Comment 4

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

Comment on attachment 485458 [details] [diff] [review]
Patch

Actually, I'm not sure we want to stretch the gradient over the height. It is likely to show banding. Let's check on device first.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Banding shot

Forgot about this bug for a bit. This does show significant banding. We'll need a new larger size image I guess(?).

Comment 6

[Madhava Enros [:madhava]]

Looks like we're going to try using a transparent gradient image bottom aligned, so that we're not covering the whole height with image.

Comment 7

[Sean Martell]

Attachment: Prefs seperator transparent gradient

updated the prefs-seperator-hdpi.png image to have a transparent background.

set the background color behind it to #f2f2f2 and should be good to go.

for the taller seperators, be sure to align the PNG to bottom.

Comment 8

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

Let's switch to Sean's new separators. We also need to make sure this works on mdpi devices, where the height is not the same pixel length as on a hdpi device.

Comment 9

[Wesley Johnston (:wesj)]

Attachment: Patch v2

This shows some banding too, but wanted to put it up. Screenshots to follow.

Comment 10

[Wesley Johnston (:wesj)]

Attachment: Addons Manager

Comment 11

[Wesley Johnston (:wesj)]

Attachment: Prefs Pane

Comment 12

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

Madhava - I'd rather have no gradients (shocker) than have the banding

Comment 13

[Wesley Johnston (:wesj)]

Attachment: Patch v3

Whoops. The old patch wasn't quite right. This doesn't get rid of banding. I also attempted two other methods, aligning the image to the bottom of the field so that it isn't stretched, and a different image with a gray background (no transparency) stretched to fit the area. All of them show banding. I can provide screen shots if we want.

Comment 14

[Wesley Johnston (:wesj)]

Attachment: Prefs Pane - Aligned to bottom

Comment 15

[Wesley Johnston (:wesj)]

Attachment: Addons Manager - Aligned to bottom

Pics with the gradient set at:

background-position: bottom left;

and no background-size attribute.

Comment 16

[Wesley Johnston (:wesj)]

Attachment: Patch v3

Patch

Comment 17

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

Comment on attachment 513492 [details] [diff] [review]
Patch v3


> /* be consistent with the size of placeitem */
> placelabel {

>-  background-image: url(images/arrowup-16.png), url("images/row-header-bg.png");
>+  background-image: url"chrome://browser/skin/images/images/arrowup-16.png"),
>+                    url("chrome://browser/skin/images/prefs-seperator-hdpi.png");

url"chrome  -> url("chrome

and since it's used for headers in general, not just in preferences:
prefs-seperator-hdpi.png -> seperator-bg-hdpi.png

lastly, what does the banding look like now?

r+ if banding is gone

Comment 18

[Wesley Johnston (:wesj)]

Attachment: Screenshot extravaganza

This does not remove banding entirely. Accoding to madhava on IRC:

08:53:33 AM - madhava: still a bit bandy
08:53:44 AM - madhava: but _way_ less so
08:53:52 AM - madhava: I'd use it -- better than flat

To be honest, I don't think the device screenshots reflect how this looks in real life either. I don't tend to notice things like this unless I am intentionally looking for them though, so I'm a bit torn. I put up a build with this: http://dl.dropbox.com/u/72157/fennec.apk (may take a few more
minutes to upload).

Comment 19

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

Tested the build on a Galaxy Tab and the banding is still quite noticeable. I am going to r- and drop this from blocking. We hoped this was a simple fix, but it doesn't seem to be. Given the other bugs on the blocker list, I need to drop this bug.

Comment 20

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

Comment on attachment 513492 [details] [diff] [review]
Patch v3

r+ was conditional on banding.

Comment 21

[BMO Automation]

Closing all opened bug in a graveyard component