975217 - Don't try to remove a banner message that doesn't exist

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Attachment: bannerremove.diff

Currently, if you pass a bad id to Home.banner.remove, it just removes the last message in the rotation. That's not good.

Comment 1

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

Comment on attachment 8379373 [details] [diff] [review]
bannerremove.diff

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

(In reply to :Margaret Leibovic from comment #0)
> Currently, if you pass a bad id to Home.banner.remove, it just removes the
> last message in the rotation. That's not good.

I'm not sure I see that - it would either delete nothing (i.e. invalid ID) or delete something it shouldn't (i.e. wrong ID).

But the validation check is a good idea in any case.

Comment 2

[:Margaret Leibovic]

(In reply to Michael Comella (:mcomella) from comment #1)
> Comment on attachment 8379373 [details] [diff] [review]
> bannerremove.diff
> 
> Review of attachment 8379373 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to :Margaret Leibovic from comment #0)
> > Currently, if you pass a bad id to Home.banner.remove, it just removes the
> > last message in the rotation. That's not good.
> 
> I'm not sure I see that - it would either delete nothing (i.e. invalid ID)
> or delete something it shouldn't (i.e. wrong ID).
> 
> But the validation check is a good idea in any case.

If we have an invalid id, we end up calling _queue.splice(-1, 1) here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/Home.jsm#142

If the index passed to splice is negative, it will begin that many elements from the end:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice#Parameters

That's the problem I noticed when testing locally.

Comment 3

[:Margaret Leibovic]

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

Comment 4

[Phil Ringnalda (:philor)]

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