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.
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 = getRecentHistory() > + m.screen.setContentList(0, m.history) > + 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) > > + 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
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
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.