660570 - Awesomebar, Bookmarks and History panes have different visual appearance in contrast to other panes in Gingerbread theme

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Attachment: Bookmarks pane

Mozilla/5.0 (Android; Linux armv7l; rv:7.0a1) Gecko/20110529 Firefox/7.0a1 Fennec/7.0a1

The following changeset [1] contains modifications to the Gingerbread theme which landed patches from Bug 659457. 

[1] http://hg.mozilla.org/mozilla-central/rev/113014cf801a

The bookmark pane is not consistent in appearance with the other panes; see attached screenshot.

Comment 2

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

This affects the entire Awesomescreen

Comment 3

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

Attachment: wip

This converts the awesomescreen (all pages, bookmarks, history, remote tabs) to gingerbread white-on-black theme..

There are some colors I need UX help:
* The blue subtitles look bad. I made them white for now
* Badge is still white-on-red
* "star" and"remote" lucky charm images in awesomescreen "all pages" might be too gray (dark) now

Screenies coming

Comment 4

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

Attachment: all pages

Comment 5

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

Attachment: bookmarks

Comment 6

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

Attachment: history

Comment 7

[Paul [pwd]]

We need to show come consistency here with the rest of the users browsing experience. To switch from a light background to a dark background is not good for the anyone's eyes. While I understand trying to match the default OS's settings, however we cannot do so at the detriment of our own usability.

Comment 8

[Madhava Enros [:madhava]]

No please - I don't want the awesomescreen lists to have black backgrounds. To many of the recognizable elements you expect in a web-browser (favicons, blue links, etc.) just don't work on a dark background like that.

As to the consistency argument, prefs screens across pretty much all of android have black backgrounds, so changing our prefs/downloads/add-ons screens is a reasonable compromise (they also look just fine on a black background).  Many apps keep a brighter white or light gray background for the lists that are their primary content -- see Twitter, Gmail, etc.  Awesomescreen content is primary for us.

Comment 9

[Madhava Enros [:madhava]]

To be clear - all the lists in the awesomescreen (all, bookmarks, history, desktop) should have white backgrounds, as in v4.

Comment 10

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

Attachment: patch

This patch does a few things:
* Keeps the awesomescreen panels black-on-white
* Changes the selection color to the Android "orange"
* Cleanup the "disabled" text color for buttons and add-on items
* Cleanup some hardcoded color: #000 and background-color: #fff by making an "inverse" set of defines

Comment 11

[Wesley Johnston (:wesj)]

I wonder if this might be a good place to use system colors now? We seem to support highlight and highlight-text on my phone (and its orangish).

Comment 12

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

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

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

Nice "grey", "gray", "#aaa", lightgrey clean up.

Asking the same question as Wes, why can't we use the system highlight color? Is it because we use custom colors for the rest of the theme and you are afraid of having a dark highlight on a dark theme if someone has a custom theme?

r+ with question answered

::: mobile/themes/core/gingerbread/browser.css
@@ +655,2 @@
>  }
>  

I wonder if this will have an impact on the bookmark-edit panel that is shown when tapping on the "edit" button of the star button popup?

@@ +766,2 @@
>  }
>  

???

@@ +859,1 @@
>  }

Why do you hardcode "white" in many places but not here?

::: mobile/themes/core/gingerbread/platform.css
@@ +321,3 @@
>    background-image: url("chrome://browser/skin/images/toggle-on.png");
>  }
>  

what does "???" means?

Comment 13

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

(In reply to comment #11)
> I wonder if this might be a good place to use system colors now? We seem to
> support highlight and highlight-text on my phone (and its orangish).

system colors will be the next step. we are not sure how do make sure some themes use the system colors and some themes don't

right now, we use prefs (in mobile.js) to kill the system colors. when we stop doing that, there might be fallout in other themes.

Comment 14

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

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

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

::: mobile/themes/core/gingerbread/browser.css
@@ +655,2 @@
>  }
>  

won't affect the dialog. only affects the richlistbox.

@@ +766,2 @@
>  }
>  

I want to remove that line. thanks.

@@ +859,1 @@
>  }

oops. I will update

Comment 15

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

pushed
http://hg.mozilla.org/mozilla-central/rev/7ade051d8cee

Comment 16

[Aaron Train [:aaronmt]]

Looks great!

Verified fixed:
Mozilla/5.0 (Android; Linux armv7l; rv:7.0a1) Gecko/20110601 Firefox/7.0a1 Fennec/7.0a1

Comment 17

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

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

low-risk, android only, been baking on trunk for a while and vastly improves the gingerbread theme

Comment 18

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

http://hg.mozilla.org/releases/mozilla-aurora/rev/366bc7bef8f1