Kiwix-android: Icon of bookmarks and history items does not change on unselecting the item

[yashk2000]

Describe the bug
After an item/s is selected in the bookmarks or history section, then on unselecting it, the icon does not change back to the default. It stays as a blue circular tick mark.

Expected behavior
The icon should change back to back it was before selection.

Steps to reproduce the behavior:

1. Go to bookmarks.
2. Select items.
3. Unselect them.
4. The original icon does not come back. Instead a blue tick mark is shown.

Screenshots

After selecting and unselecting

Original

Environment

• Version of Kiwix Android : 3.1.3
• Device : Redmi K20
• OS version : Android 9(Pie)

[GearGit]

I would like to work on this issue

[macgills]

@GearGit I belive @yashk2000 opened this ticket with the intention of working on it. I'll give yash first chance to work on it or they can pass it on to you.

@GearGit you can also have a look at our good first issues and see if anything piques your interest.

[kelson42]

I confirm the bug. @yashk2000 Do you confirm working on ti?

[yashk2000]

Yes I am working on it :+1:

[kelson42]

@yashk2000 Any PR, even in draft?

[yashk2000]

@kelson42 I have started working on the issue. I was working on another issue until yesterday that's why I didn't give an update on this.

[yashk2000]

@kelson42 @macgills I am trying to work on this issue, but I am unable to figure out how to fix it properly. As far as I could figure out, the items in the history list are not being removed properly due to which the adapter changes/keeps the icon as the blue tick mark.

[macgills]

I found the issue.

First I set a breakpoint in this method to assert that the correct method was being called

  private void toggleSelection(ImageView favicon, HistoryListItem.HistoryItem history) {
    if (deleteList.remove(history)) {
      ImageViewExtensionsKt.setBitmapFromString(favicon, history.getFavicon());
    } else {
      favicon.setImageDrawable(
        ContextCompat.getDrawable(this, R.drawable.ic_check_circle_blue_24dp));
      deleteList.add(history);
    }
    actionMode.setTitle(getString(R.string.selected_items, deleteList.size()));

    if (deleteList.size() == 0) {
      actionMode.finish();
    }
  }

On deselect the correct codepath was taken which leads us to

ImageViewExtensionsKt.setBitmapFromString(favicon, history.getFavicon());

fun ImageView.setBitmapFromString(string: String?) {
setBitmap(Base64String(string))
}

fun ImageView.setBitmap(base64String: Base64String) {
  if (tag != base64String) {
    base64String.toBitmap()
      ?.let {
        setImageBitmap(it)
        tag = base64String
      }
  }
}

at this line if (tag != base64String) { I could inspect the tag in the debugger

Surprise surpise, it is already set.
The error was I was thinking we would never change the image for a favicon, this was to stop us loading the image over and over again while scrolling a list but if it is interfering with this functionality so we can probably just remove retrieving/setting the tag

[yashk2000]

@macgills thanks! I saw the function, but I never thought of checking the tag condition.