Last Comment Bug 660570 - Awesomebar, Bookmarks and History panes have different visual appearance in contrast to other panes in Gingerbread theme
: Awesomebar, Bookmarks and History panes have different visual appearance in c...
Status: VERIFIED FIXED
: regression
Product: Fennec Graveyard
Classification: Graveyard
Component: General (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: Firefox 7
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
:
Mentors:
: 660571 (view as bug list)
Depends on: 661284
Blocks: 659457
  Show dependency treegraph
 
Reported: 2011-05-29 20:33 PDT by Aaron Train [:aaronmt]
Modified: 2011-06-02 19:48 PDT (History)
6 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Bookmarks pane (25.36 KB, image/png)
2011-05-29 20:33 PDT, Aaron Train [:aaronmt]
no flags Details
wip (6.51 KB, patch)
2011-05-29 22:26 PDT, Mark Finkle (:mfinkle) (use needinfo?)
no flags Details | Diff | Splinter Review
all pages (48.09 KB, image/png)
2011-05-29 22:27 PDT, Mark Finkle (:mfinkle) (use needinfo?)
no flags Details
bookmarks (37.08 KB, image/png)
2011-05-29 22:28 PDT, Mark Finkle (:mfinkle) (use needinfo?)
no flags Details
history (35.20 KB, image/png)
2011-05-29 22:28 PDT, Mark Finkle (:mfinkle) (use needinfo?)
no flags Details
patch (20.14 KB, patch)
2011-05-31 09:55 PDT, Mark Finkle (:mfinkle) (use needinfo?)
21: review+
blassey.bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Aaron Train [:aaronmt] 2011-05-29 20:33:59 PDT
Created attachment 535986 [details]
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 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-29 22:12:50 PDT
*** Bug 660571 has been marked as a duplicate of this bug. ***
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-29 22:14:26 PDT
This affects the entire Awesomescreen
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-29 22:26:11 PDT
Created attachment 536006 [details] [diff] [review]
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?) 2011-05-29 22:27:52 PDT
Created attachment 536007 [details]
all pages
Comment 5 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-29 22:28:19 PDT
Created attachment 536008 [details]
bookmarks
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-05-29 22:28:47 PDT
Created attachment 536009 [details]
history
Comment 7 Paul [pwd] 2011-05-31 05:39:56 PDT
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] 2011-05-31 07:42:36 PDT
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] 2011-05-31 07:44:27 PDT
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?) 2011-05-31 09:55:53 PDT
Created attachment 536327 [details] [diff] [review]
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) 2011-05-31 10:31:39 PDT
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) 2011-05-31 10:41:31 PDT
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?) 2011-05-31 11:11:10 PDT
(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?) 2011-05-31 11:17:52 PDT
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?) 2011-05-31 13:09:05 PDT
pushed
http://hg.mozilla.org/mozilla-central/rev/7ade051d8cee
Comment 16 Aaron Train [:aaronmt] 2011-06-01 06:28:07 PDT
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?) 2011-06-02 10:10:50 PDT
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?) 2011-06-02 19:48:13 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/366bc7bef8f1

Note You need to log in before you can comment on or make changes to this bug.