[Roku] Removing an entry from history will only take effect after restarting the Roku app

VERIFIED FIXED

Status

()

Firefox for Android
Screencasting
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: Flaviu Cos, Assigned: mfinkle)

Tracking

Trunk
ARM
Android
Points:
---

Firefox Tracking Flags

(fennec33+)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
Environment: 
Casting device: Roku 3;

Steps to reproduce:
1. Open the Firefox app on Roku;
2. Go to Recent History;
3. Press the menu (*) button on the remote;
4. Choose 'Remove from history' option from the list;
5. Chose 'Yes' to confirm;
6. Press 'Ok' button on the remote to go back the history page.

Expected result:
The history entry is successfully removed.

Actual result:
The video is still present in history, but it will be deleted after restarting the Roku app.

Updated

4 years ago
tracking-fennec: --- → ?
(Assignee)

Updated

4 years ago
Assignee: nobody → mark.finkle
tracking-fennec: ? → 33+
Created attachment 8485456 [details] [diff] [review]
remove-history v0.1

I realize this is mostly Greek to you, but the gist of the patch is:
1. roGridScreen data is backed in an array of arrays (m.history)
2. roGridScreen does not have a way to remove an individual item itself. You need to remove it from the backing array of arrays (m.history) and then reload the data into roGridScreen
3. You also need to update the visibility of the datasets. If the recent history dataset is empty, don't show it in the grid.

We already did #3 when initializing the grid, so I pulled the function out and I'm calling it from do places now.

I also found a bug where we allowed the user to attempt to remove the built-in default items, but those are hard coded. I added some code to figure out which history items are "removeable" and then adjust the popup menu with the right actions.

I tested this patch 3 or 4 ways and it seems to be working fine now.
Attachment #8485456 - Flags: review?(wjohnston)
Comment on attachment 8485456 [details] [diff] [review]
remove-history v0.1

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

::: app/source/recentHistory.brs
@@ +113,5 @@
> +
> +        ' Update the screen
> +        m.history[0] = getRecentHistory()
> +        m.screen.setContentList(0, m.history[0])
> +        m.setSelectedItem()

This method feels like it should take a parameter. Maybe selectNextItem() or something would be better.

@@ +140,4 @@
>                  HDPosterUrl: video.poster
>                  SDPosterUrl: video.poster
>                  videoURL: video.url
> +                removeable: true

s/removeable/removable/
Attachment #8485456 - Flags: review?(wjohnston) → review+
(In reply to Wesley Johnston (:wesj) from comment #2)

> > +        m.screen.setContentList(0, m.history[0])
> > +        m.setSelectedItem()
> 
> This method feels like it should take a parameter. Maybe selectNextItem() or
> something would be better.

Makes sense. I went with selectInitialItem.

> >                  videoURL: video.url
> > +                removeable: true
> 
> s/removeable/removable/

Oops, thanks
https://github.com/mozilla/firefox-roku/commit/a039dae129f76f11ce6c78bb2022d96d46900df5
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 5

4 years ago
Verified as fixed in builds:
- Firefox Nightly for Roku v1.1
- Firefox beta for Roku v1.0
Device: Roku 2.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.