For #27975: Ignore current tab when forcing inactive

[DreVla]

This feature is only available in debug and it is used by developers and qa to test inactive tabs without having to change device time or wait 15 days for a tab to become inactive! This specific crash was caused when a user forced the current tab to be inactive, which led to the tab at the same time being inactive but still updating its metadata when certain actions we're done (closing the tab, switching to another tab). Since the crash was caused by having the current tab set as inactive while the user was still able to see it, now we ignore the current tab if it is selected to be set as inactive.

inactive_tab_fix.mp4

Pull Request checklist

• Tests: This PR includes thorough tests or an explanation of why it does not
• Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
• Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

QA

• QA Needed

To download an APK when reviewing a PR (after all CI tasks finished running):

1. Click on Checks at the top of the PR page.
2. Click on the firefoxci-taskcluster group on the left to expand all tasks.
3. Click on the build-debug task.
4. Click on View task in Taskcluster in the new DETAILS section.
5. The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

Fixes #27975

[t-p-white]

Nice, this has stopped the crashing 🙂
Being able to select the current tab and 'make inactive' but not actually making it 'inactive' doesn't feel like the right UX - I wonder if we should also remove/disable the ability to make a current tab 'inactive' altogether?
Edit: Although given this is just for debug builds perhaps that's fine

[DreVla]

Nice, this has stopped the crashing 🙂 Being able to select the current tab and 'make inactive' but not actually making it 'inactive' doesn't feel like the right UX - I wonder if we should also remove/disable the ability to make a current tab 'inactive' altogether? Edit: Although given this is just for debug builds perhaps that's fine

We cannot really disable the ability to make a current tab 'inactive' since we do not know what the user wants to do when selecting the current tab in the tabs tray (maybe he wants to delete it or add it to a collection). I would say that considering this is just a debug feature, there is no need for further complications in the code.

[t-p-white]

I would say that considering this is just a debug feature, there is no need for further complications in the code.

Understood. Code changes lgtm 👍

[rahulsainani]

❓ Question: Should we add tests for it?

💡 Suggestion: Maybe we can add checks that it should only work on debugging mode and does nothing in other modes, not for this PR.

[DreVla]

I was thinking of adding tests but considering it is a feature used only for debugging, I didn't really see them necessary. Not sure if the suggestion is for the tests or for the feature. The check for the force inactive feature is implemented here: https://searchfox.org/mozilla-mobile/source/fenix/app/src/main/java/org/mozilla/fenix/tabstray/browser/SelectionMenu.kt#48

[rahulsainani]

Fair point, although seeing that it did break, and you fixed it here, even for debugging adding the tests would avoid this from happening in the future, i leave it to your better judgement :)
Suggestion was for the feature, the check is somewhere else and from reading the comments it seemed like action can be used by mistake, adding a check there would prevent that. Again, not essential for this PR, but might prevent this super user feature from leaking into the release ;)

[DreVla]

Kudos to @Mugurell for helping, three new tests have been added which would ensure future stability for this feature.