773293 - Checkboxes in Send Tab To Device activity are using non-OS theme

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P3)

Description

[Aaron Train [:aaronmt]]

Attachment: Nightly (07/12) Screenshot

See screenshot, I'm on Jellybean but the checkboxes are for Gingerbread.

--
Nightly (07/12)
Samsung Galaxy Nexus (Android 4.1.1)

Comment 1

[Aaron Train [:aaronmt]]

(If it's not clear, they're supposed to be blue http://compixels.com/wp-content/uploads/2012/01/ICS-Browser+-Advanced-Settings.png).

Comment 2

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

Is it like this for ICS too?

Comment 3

[Richard Newman [:rnewman]]

*blink*

Comment 4

[Aaron Train [:aaronmt]]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Is it like this for ICS too?

Yes

Comment 5

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

Attachment: Patch against m-i

Hi sriram, how do you feel about adding Gecko.Dialog to resources/values/themes.xml?  I would like to add it so that SyncTheme can inherit from Gecko (on phone) or Gecko.Dialog (on tablet, sees mocks at Bug 767179) and that future Gecko theming propagates to the Sync UI as much as possible.

Comment 6

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

Not a hard dep, but I'd like to land this after Bug 767179 lands.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 654807 [details] [diff] [review]
Patch against m-i

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

This looks good to me.

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 654807 [details] [diff] [review]
Patch against m-i

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

::: mobile/android/base/resources/values-v11/themes.xml
@@ +9,5 @@
>          <item name="android:windowContentOverlay">@null</item>
>          <item name="android:windowActionBar">false</item>
>          <item name="android:windowNoTitle">true</item>
>      </style>
>      

Trailing whitespace.

@@ +15,5 @@
> +        <item name="android:windowContentOverlay">@null</item>
> +        <item name="android:windowActionBar">false</item>
> +        <item name="android:windowNoTitle">true</item>
> +    </style>
> +    

Trailing whitespace.

::: mobile/android/base/resources/values/themes.xml
@@ +13,5 @@
> +    <style name="Gecko.Dialog" parent="@android:style/Theme.Dialog">
> +        <item name="android:windowNoTitle">true</item>
> +        <item name="android:windowContentOverlay">@null</item>
> +    </style>
> +    

Aaaarg

Comment 9

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/14ea772f1745
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce60fb4481d5

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/14ea772f1745
https://hg.mozilla.org/mozilla-central/rev/ce60fb4481d5

Comment 11

[Aaron Train [:aaronmt]]

Looks nice!

Uplift or ride it?

Comment 12

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

This should probably move in lockstep with Bug 767179 (tablet UI).  Let's get some feedback on that, and then consider uplifting both.

Comment 13

[Aaron Train [:aaronmt]]

Nit: Why are row highlights orange (not the default system theme)?

Comment 14

[Richard Newman [:rnewman]]

(In reply to Aaron Train [:aaronmt] from comment #13)
> Nit: Why are row highlights orange (not the default system theme)?

As I understand it, we now share Fennec's styles.