We need some way to ensure enough contrast between the toolbar's background and the the 'X' button. See screenshot.

Saw this in April, said in IRC "I meant to file a bug", then didn't :D Fail on my part!

Looking at Desktop, they seem to be able to detect whether a theme is light or dark, and actually reverses the icon colours when the background is dark. http://cl.ly/image/3Z372B2Z140g

Turns out, this has all happened before: bug 816114, bug 822421 and bug 822422. Bug 822422 deals with flipping resources (images) based on the theme.

Whoa. I'm starting to think Finkle and Bugzilla are the same thing.

Mike, can you have a look?

I converted the X button to #FFFFFF - what do you think, Ian? I suppose this can still be a problem on both light and dark backgrounds though.

I'm not sure if the white assests should be called "light" for their color, or "dark" for the themes they're expected to be used on.

Note to self - uplift to 31 to match the edit cancel button.

Comment on attachment 8434374 [details] [diff] [review] Use light cancel edit button with dark lightweight themes. Missing close_edit_mode_selector.xml ?

(In reply to Michael Comella (:mcomella) from comment #6) > Created attachment 8434361 [details] > Dark theme, light X > > I converted the X button to #FFFFFF - what do you think, Ian? This looks good > I suppose this can still be a problem on both light and dark backgrounds > though. No solution will be perfect here. And I don't want to get into using crazy icon borders or drop shadows to compensate for this.

Added close_edit_mode_selector as per comment 10, and spare Lucas another review.

Comment on attachment 8434450 [details] [diff] [review] Use light cancel edit button with dark lightweight themes. >diff --git a/mobile/android/base/resources/drawable/close_edit_mode_selector.xml b/mobile/android/base/resources/drawable/close_edit_mode_selector.xml >+ <item gecko:state_dark="true" >+ android:drawable="@drawable/close_edit_mode_light"/> >+ >+ <item android:drawable="@drawable/close_edit_mode"/> Can we rename the close_edit_mode -> close_edit_mode_light Can we rename the close_edit_mode_light ->close_edit_mode_dark so the suffix matches the state?

Updated as per comment 13.

Comment on attachment 8434507 [details] [diff] [review] Use light cancel edit button with dark lightweight themes. Review of attachment 8434507 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. ::: mobile/android/base/toolbar/BrowserToolbar.java @@ +1453,5 @@ > StateListDrawable stateList = new StateListDrawable(); > stateList.addState(PRIVATE_STATE_SET, getColorDrawable(R.color.background_private)); > stateList.addState(EMPTY_STATE_SET, drawable); > > + editCancel.onLightweightThemeChanged(); Nit: put this under `setBackgroundDrawable(stateList);` to keep the stateList code together.

Updated for nits (this is the one pushed to fx-team).

Comment on attachment 8435175 [details] [diff] [review] Use light cancel edit button with dark lightweight themes. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 965548 User impact if declined: Users with dark lightweight themes will not be able to see the cancel editing mode button clearly Testing completed (on m-c, etc.): Local Risk to taking this patch (and alternatives if risky): Low - we've essentially just casted some views so in the worst (and unlikely) case, these casts fail, throw an exception, and the browser dies (but it's an easy fix). String or IDL/UUID changes made by this patch: None

Verified as fixed in build 32.0a1 (2014-06-06) Device: Asus Transformer TF101 (Android 4.0.3)

Needs a branch patch for Aurora uplift. Note that the merge to beta is on Monday, so this needs to land ASAP.

The cancel button isn't available on tablet so I added some guards; bnich, double-check me please!

Comment on attachment 8435175 [details] [diff] [review] Use light cancel edit button with dark lightweight themes. bzexport made me do it.

For aurora.

Verified as fixed in 31.0a2 (2014-06-09) on phones. On tablets (aurora), the cancel edit mode isn't available, can we consider this verified and fixed?

Verified as fixed in: Build: Firefox for Android 31 Beta 4 Device: Asus Transformer Pad TF300T (Android 4.2.1).

(In reply to cristina.madaras from comment #27) > Verified as fixed in: > Build: Firefox for Android 31 Beta 4 > Device: Asus Transformer Pad TF300T (Android 4.2.1). Sorry! My mistake, I tested this on Motorola Razr (Android 4.0.4), not on Asus. On tablets (Beta), the cancel edit mode isn't available, Bug 997477.