1199015 - Remove alert_app_animation drawables

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

In bug 1186020, we convert the download animation icon assets to white because it appears grey on some devices – I'd expect alert_app_animation to have the same problem.

The animation appears to occur when we update outdated apps [2], which doesn't seem like it'd take long enough to warrant an animation (or if we even still use this often) so maybe we can remove it and save some APK space. 

Finkle, thoughts? How can I test this?

[1]: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/drawable-xhdpi/alert_app_animation_1.png
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/WebappManager.jsm?rev=3c6b38653a58#470

Comment 1

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

The animation was added in bug 970203 by Martyn. He might remember how to get updates to work, or at least trigger the action for testing.

Comment 2

[Martyn Haigh (:mhaigh)]

I can't honestly remember how to go about triggering a webapp update.  Myk may have some idea or links to test pages to help

Comment 3

[Myk Melez [:myk] [@mykmelez]]

(In reply to Martyn Haigh (:mhaigh) from comment #2)
> I can't honestly remember how to go about triggering a webapp update.  Myk
> may have some idea or links to test pages to help

It used to be possible to initiate a manual check for updates via some UI on about:apps, but there isn't any UI for doing so now that about:apps has been removed.


(In reply to Michael Comella (:mcomella) from comment #0)
> The animation appears to occur when we update outdated apps [2], which
> doesn't seem like it'd take long enough to warrant an animation (or if we
> even still use this often) so maybe we can remove it and save some APK
> space. 

It doesn't necessarily take that long, a few seconds at most on a fast network connection.  But that's still long enough to be confusing for the user who initiates an update check and then doesn't see anything happening.

However, as noted above, there's no way for the user to initiate an update check anymore.  And we only show that animation when the user initiates a check (i.e. when *userInitiated* is `true`).  We don't show anything when Fennec initiates its periodic (daily) update check in the background.  So we never show that animation anymore, and we can remove it.

Comment 4

[Matherin]

Hello all, I was hoping to be assigned to this bug if possible! And let me reiterate what I read above to be sure I am making the correct change. Do we just want to remove the line containing alert_app_animation drawables or would you like the entire logic removed? By entire logic I mean the  if (userInitiated) structure.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Matherin from comment #4)
> Hello all, I was hoping to be assigned to this bug if possible! And let me
> reiterate what I read above to be sure I am making the correct change. Do we
> just want to remove the line containing alert_app_animation drawables or
> would you like the entire logic removed? By entire logic I mean the  if
> (userInitiated) structure.

Hey Matherin. We're actually removing our Web App runtime, which will remove these assets and their use altogether. Perhaps you'd like to tackle bug 1245952 instead?

Comment 6

[Brendan Dahl [:bdahl]]

Attachment: 0001-Bug-1199015-Remove-alert_app_animation-drawables.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c7560f36373

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8726549 [details] [diff] [review]
0001-Bug-1199015-Remove-alert_app_animation-drawables.patch

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

Splinter may be hiding some deletions from me, but it appears you can also delete the `.../drawable-hdpi*/alert_app*.png` assets. See:
  https://mxr.mozilla.org/mozilla-central/find?string=alert_app&tree=mozilla-central&hint=

Nice find on the UnusedResourcesUtil. :)

Comment 8

[Brendan Dahl [:bdahl]]

Attachment: 0001-Bug-1199015-Remove-alert_app_animation-drawables.patch

Splinter was hiding some removals, but I missed the ones outside of xhdpi. Carrying r+ forward

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/b7d800609f31

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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