977643 - Now that android has a colorpicker, it shouldn't have a special pref("dom.forms.color", true) line in its prefs file

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Bug 875750 added a color picker implementation for Android, bringing it up to parity with Windows / Mac / Linux.

It toggled the line
 pref("dom.forms.color", false);
in mobile/android/app/mobile.js to "true", when really it should have just deleted the line (so that we can take the default value from all.js).

Comment 1

[Arnaud Bienner]

Attachment: android_input_type_color_pref.patch

Indeed, we decided to have this pref on by default, then overridden for platforms where is was missing. So, this line isn't needed.
Moreover, the comment was wrong indeed.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 8383088 [details] [diff] [review]
fix

Oh, you beat me to it :)

(Darned attachment-creation page, not showing midair patches)

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 8383085 [details] [diff] [review]
android_input_type_color_pref.patch

Requesting beta/aurora approval.

On Aurora, this has no effect, since we'll just get the same value from libpref's all.js.

On Beta, this is required in order for bug 977301 (which is disabling this feature) to take effect on Android.  So this is sort of a helper-bug for that bug. (just split out into this bug because it's a logically-separate [though prerequisite] change)

(Side note: a part of me thinks that maaaaybe we should just leave it enabled on Android, but I'm leaning towards thinking we should just disable it everywhere, because several of the bugs that are making us disable it (listed in bug 977301 comment 0) are non-platform-specific (bug 977038, bug 977029, and bug 949403))

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Daniel, this patch should land in m-c first, right ?

Comment 6

[Daniel Holbert [:dholbert]]

Yes, and I intend to land them in the correct order.

I'm just preemptively requesting approval since I'm told the schedule for getting things into beta is tight, and this is required as part of bug 977301 (on beta).

Comment 7

[Daniel Holbert [:dholbert]]

Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/b4826a1c1a27

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b4826a1c1a27

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8383085 [details] [diff] [review]
android_input_type_color_pref.patch

From my read on the comments in but 977301 it looks like the best plan is to not ship this at all in FF28 and try to get it together on FF29 with longer bake time on Beta.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/29d8fd6fe486

Leaving the beta28 landing to dholbert pending bug 977301.

Comment 11

[Daniel Holbert [:dholbert]]

Thanks, Ryan!

Landed on Beta28: https://hg.mozilla.org/releases/mozilla-beta/rev/0faa97a9c61c

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/0faa97a9c61c