844935 - Temporarily remove homescreen wallpaper feature due to impact on auto updates

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Alex Keybl [:akeybl]]

Kevin brought up a new permission being necessary for bug 715736 in FF20. This prevents automatic updates, so we're wondering if we should delay this permissions change till a more major permissions change is made.

Comment 1

[Alex Keybl [:akeybl]]

Margaret - can you help disable this feature and remove the permission? Nobody has made the case for retaining this feature in FF20, given the impact to updates.

Tracking for FF21 as well, since it's unclear when our next critical permissions change is necessary.

Comment 2

[:Margaret Leibovic]

(In reply to Alex Keybl [:akeybl] from comment #1)
> Margaret - can you help disable this feature and remove the permission?
> Nobody has made the case for retaining this feature in FF20, given the
> impact to updates.

Sure. Just on beta?

Comment 3

[:Margaret Leibovic]

Attachment: patch to disable wallpaper feature (applies on beta)

Comment 4

[:Margaret Leibovic]

Comment on attachment 719204 [details] [diff] [review]
patch to disable wallpaper feature (applies on beta)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 715736
User impact if declined: permissions change and manual update
Testing completed (on m-c, etc.): local build
Risk to taking this patch (and alternatives if risky): n/a
String or UUID changes made by this patch: n/a

Comment 5

[Kevin Brosnan [Ex-Mozilla]]

Have we heard back from Karen and development leads yet? We've already gone to build for beta 2. We should have until next Tuesday mid-day Pacific to land this for beta 3.

Comment 6

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

R+ on removing the feature until we have a solid "permission batch". Please disable on Aurora too.

Comment 7

[:Margaret Leibovic]

Attachment: patch for aurora

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 715736
User impact if declined: permissions change and manual update
Testing completed (on m-c, etc.): local build
Risk to taking this patch (and alternatives if risky): n/a
String or UUID changes made by this patch: n/a

Comment 8

[bhavana bajaj [:bajaj]]

Comment on attachment 719204 [details] [diff] [review]
patch to disable wallpaper feature (applies on beta)

Approving the patch for uplift & request to QA with verification ensuring the feature is disabled .

Comment 9

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3c791760da94
https://hg.mozilla.org/releases/mozilla-beta/rev/09de427dd00f

Comment 10

[Adrian Tamas (:AdrianT)]

Option is no longer available on Aurora 21.0a2 2013-03-14 and Firefox Mobile 20 beta 5 on the Smasung Galaxy Tab 2 7.0 (Android 4.1.1)

Comment 11

[:Margaret Leibovic]

Marking this bug as fixed, since it was about disabling the feature on 20/21. I filed bug 851170 as a follow-up.

Comment 12

[Alex Keybl [:akeybl]]

We haven't decided whether to do this for FF23 again, but we may still want to track for now.

Comment 13

[Adrian Tamas (:AdrianT)]

The option is active on Firefox Mobile 22 beta 1

Comment 14

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

Margaret - Can we get this backed out of fx22 (beta) and fx23 (aurora)? And should we move to a build flag?

Comment 15

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #14)
> Margaret - Can we get this backed out of fx22 (beta) and fx23 (aurora)? And
> should we move to a build flag?

I guess changing the confvars variable didn't get onto someone's list? Bug 851170 is in 22, so we just need to flip that variable.

Comment 16

[Alex Keybl [:akeybl]]

Attachment: Disable wallpaper for FF22

Comment 17

[Alex Keybl [:akeybl]]

https://hg.mozilla.org/releases/mozilla-beta/rev/285b70591ec6

Comment 18

[Aaron Train [:aaronmt]]

I still see this permission on mozilla-beta, margaret, can you confirm?

http://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/22.0b3-candidates/build1/android/multi/fennec-22.0b3.multi.android-arm.apk

Comment 19

[:Margaret Leibovic]

(In reply to Aaron Train [:aaronmt] from comment #18)
> I still see this permission on mozilla-beta, margaret, can you confirm?
> 
> http://ftp.mozilla.org/pub/mozilla.org/mobile/candidates/22.0b3-candidates/
> build1/android/multi/fennec-22.0b3.multi.android-arm.apk

Yeah, I can confirm this feature isn't disabled here :(

I can see that's akeybl's change is definitely in the beta tree, so I'll need to figure out what's going wrong here. I'm investigating now.

Comment 20

[:Margaret Leibovic]

Oops, didn't mean to set that needinfo.

Comment 21

[:Margaret Leibovic]

Attachment: Actually disable wallpaper feature for FF22

I failed at reviewing that last patch. Our tests are for "non-empty value", not truthiness.

This patch is for beta.

Comment 22

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-beta/rev/c9a8e2bea80a

This worked for me locally, but let's make sure to verify this once builds are ready.

Is this going to go on some checklist somewhere for things to do on the next uplift to make sure this will be fixed for 23 when it goes to beta?

Comment 23

[Phil Ringnalda (:philor)]

I fixed your permaorange in https://hg.mozilla.org/releases/mozilla-beta/rev/2507883ac7e4 by disabling testWebContentContextMenu, since we actually disabled in in 23 so that we could run robocop on Pandas (pay no attention to the fact that we're running robocop on Pandas on 22, where it and the other six things we disabled with it are not disabled).

Comment 24

[Lukas Blakk [:lsblakk] use ?needinfo]

We're on beta now - Margaret can you check on this and submit a patch for beta uplift?

Comment 25

[:Margaret Leibovic]

(In reply to [e-mail] [:lsblakk] from comment #24)
> We're on beta now - Margaret can you check on this and submit a patch for
> beta uplift?

There shouldn't be anything to do, this was fixed by bug 879350. And I just verified that there's no wallpaper permission requested in the Beta installed on my phone.