759120 - History list doesn't update after you remove the last history item using context menu

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul Feher]

Nightly Fennec 15.0a1 (2012-05-27)
Device: HTC Desire Z 
OS: Android 2.3.3

Steps to reproduce:
1. Make sure you have entrees in the History section.
2. Tap the URL bar to access the History tab.
3. Long tap on one entree for the context menu to appear.
4. Repeat step 4 and remove all entrees using the remove button.

Expected:
A toast notification appears whit the message "Page Removed" and all entrees from the History section are removed. 

Actual:
If there is only one item in the history section the toast notification appears but the item is still present. Only after going back and accessing another time the awesome-menu the History tab is updated and the entree is removed.

Comment 1

[Aaron Train [:aaronmt]]

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/LocalBrowserDB.java#263

I wonder if BrowserDB.removeHistoryEntry is receiving a null ID or something.

Looks like a Margaret bug :)

Comment 2

[Aaron Train [:aaronmt]]

Oh, this looks like the operation is not executed right away because when I exit out of the AwesomeScreen and return back to the History tab, the item is removed.

Comment 3

[:Margaret Leibovic]

The problem here is that the ContentObserver makes a new HistoryQueryTask to update the list, and that just bails if there are no items in history:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AwesomeBarTabs.java#540

Ideally, we should fix the "FIXME" in there and display some message when there is no history to show, but as a solution to this specific problem, we should empty the list.

Comment 4

[:Margaret Leibovic]

Attachment: patch

Since we're not actually handling the empty history case, I think we should just go ahead and set up the history list adapter with empty lists.

Also, I'm not sure if there's a way to clear out the contents of the list view other than just changing the adapter.

Comment 5

[Lucas Rocha (:lucasr)]

Comment on attachment 630229 [details] [diff] [review]
patch

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

::: mobile/android/base/AwesomeBarTabs.java
@@ -550,5 @@
>              return HistorySection.OLDER;
>          }
>  
>          protected void onPostExecute(Pair<GroupList,List<ChildrenList>> result) {
> -            // FIXME: display some sort of message when there's no history

Please file a bug to track this FIXME.

Comment 6

[:Margaret Leibovic]

(In reply to Lucas Rocha (:lucasr) from comment #5)

> ::: mobile/android/base/AwesomeBarTabs.java
> @@ -550,5 @@
> >              return HistorySection.OLDER;
> >          }
> >  
> >          protected void onPostExecute(Pair<GroupList,List<ChildrenList>> result) {
> > -            // FIXME: display some sort of message when there's no history
> 
> Please file a bug to track this FIXME.

I'm not sure that we even need to show a message, but I filed bug 762086 to track the issue and get some UX input.

Comment 7

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f0ed50cedde

Comment 8

[:Margaret Leibovic]

Comment on attachment 630229 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): this problem has always existed, but bug 671131 exposed it, so this patch just needs to be on aurora, not beta
User impact if declined: removing the last history item in the history tab will look like it didn't work because the list won't update to be empty
Testing completed (on m-c, etc.): just landed on inbound
Risk to taking this patch (and alternatives if risky): low-risk, just removes special-case early return for an empty history
String or UUID changes made by this patch: n/a

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7f0ed50cedde

Comment 10

[Aaron Train [:aaronmt]]

Looks good on inbound, verified fixed on 16.0a1

Comment 11

[:Margaret Leibovic]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b5fc0b2d27e7