Fix #4026, #4027, #4040, #4076, #4287, #4319, #4329: Fix miscellaneous bugs found while testing alpha MR4

[BenHenning]

Explanation

Fixes #4026
Fixes #4027
Fixes #4040
Fixes #4076
Fixes #4287
Fixes #4319
Fixes #4329

This PR fixes a bunch of different issues, but specifically:

• It removes the 'Please continue' answer from being rendered for 'Continue' interaction states (plus fixes any cases where other answers are empty, though this shouldn't generally happen in practice).
• It fixes a bunch of image sizing and cut-off issues (explained in more detail below).
• It fixes Unable to access Admin profile and Admin Controls #4040 (though, only for proto loading) for rare edge cases where topics change across app installs.
• Unable to move past a card in Chapter 6 (Fractions) #4026 (kind of): The fix is actually on a private repository branch, but it did unearth a long-standing issue with image region selection that caused answer submission to fail for certain image cases (also explained in more detail below).

Image fixes

There are images below demonstrating some of the problems and what they look like with the fix in place.

There were three categories of issues that needed to be fixed:

1. The logic for cropping images wasn't set up to support certain parent layout types, so sometimes it just wasn't even activating.
2. Sizing images based on pixels is likely to result in problems since it means images will have different physical sizes depending on the device density. The workaround is to use dp (where we're now translating 1:1 from image pixels to dp). This is a reasonably medium-term workaround, but longer term we'll need to have a way to properly define dp for Android-supported images.
3. Both of the above didn't work for some SVGs because the dimensions were either not correct, or outright missing from the SVG file. This became complicated: we have at least three different places to source the image's dimensions: its filename (reported by Oppia web), its document tag in the SVG (which can omit these attributes), or the SVG's viewport dimensions (which is sometimes too large). Due to these complexities, the app now uses all three of these sizes as heuristics to compute a larger (by 50%) rendering space. It then renders the SVG into this space and uses pixel boundary finding to reduce the image to the smallest possible space that it can consume. This is very computationally (and sometimes memory) expensive, but it's set up such that Glide will cache the result so subsequent loads are much faster (and perform better since the images are now being rendered as bitmaps instead of pictures).

Image region interaction issue

It's not entirely clear to me after investigating why this issue seems to affect alpha assets but not the local prototype exploration. Nevertheless, the problem is that image regions may not compute correctly when translated to views which makes the interaction impossible to get past (we didn't notice this before because the image itself wasn't being pulled in until #4026 was fixed). The reasonable workaround for the sizing issue was to ensure Android was explicitly told to relayout such that it would definitely recompute the region views' dimensions (https://stackoverflow.com/a/42430695/3689782 explains the flow in much more detail, and there's a detailed comment in ClickableAreasImage).

Notes on testing and verifications

I mostly focused on introducing regression tests for bugs that I could, but some aren't possible:

• Image region currently can't be tested in Robolectric, and it's difficult to test the race condition flows in Espresso (and very likely impossible to do so reliably).
• The image pipeline can't be tested due to the nature of how it integrates with Glide, and we don't have a solution to make it testable yet.
• AssetRepositoryImpl doesn't have a test suite, and I didn't want to add that in this PR due to many other priorities.

Beyond that, however, I did add tests for the other parts of the PR. I also verified that the new tests in both TopicListControllerTest and StateFragmentTest correctly failed without their corresponding fixes in place. Finally, I manually verified the continue button behavior, the crash, and image behaviors on the branch against develop to verify that the fixes are working as expected.

Essential Checklist

• The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• Any changes to scripts/assets files have their rationale included in the PR explanation.
• The PR follows the style guide.
• The PR does not contain any unnecessary code changes from Android Studio (reference).
• The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
• The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

Screenshots

Images cut-off and/or too small (without fixes)	Same images with the fixes from this PR.

Images showing the change in continue button behavior:

Previous behavior (showing 'continue' button answer)	New behavior (no answer is shown for 'continue')

Note that I didn't add localization cases or the accessibility video since neither should be affected by the UI fixes introduced in this PR.

Espresso test results

StateFragmentTest results:

Note that I only ran StateFragmentTest on Espresso since:

• While questions is affected by the continue response change, questions never use this interaction.
• StateFragmentLocalTest wasn't updated directly (since the new tests went in StateFragmentTest), and it was verified as passing on Robolectric (given it can't run on Espresso).

[BenHenning]

Issues this will fix:

• Unable to move past a card in Chapter 6 (Fractions) #4026 (kind of--the fix is technically in the script, but I'll explain it in this PR & attach it to the submission of this PR)
  • TODO: mention that this uncovered a long-standing issue with image region selection that needed to be fixed
• Image size too small or illegible #4027 specifically:
  • Image cut offs due to incorrect margin calculations for block images within content text views
  • Image cut offs due to an issue in AndroidSVG (for SVGs missing a document size)
  • Image sizing (by ensuring block images are generally rendered using physical units instead of pixels)
• Unable to access Admin profile and Admin Controls #4040
  • TODO: Mention this is only fixed for proto loading (since it doesn't seem necessary for the JSON pipeline as that's only used by developers)
• Continue button changes to a conversation message when going backwards in a chapter #4076

#4076 isn't included yet, but will be in a follow-up commit.

Once these are all re-verified as implemented, I'll need to audit this PR and clean it up in terms of code layout, architecture, and readability. Then, tests will need to be added & the PR title/description finalized before this can be sent for review.

[oppiabot]

Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[oppiabot]

Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[BenHenning]

Still active.

[BenHenning]

@anandwana001 PTAL.

[anandwana001]

LGTM

[oppiabot]

Unassigning @anandwana001 since they have already approved the PR.

[oppiabot]

Hi @BenHenning, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!

[oppiabot]

Hi @BenHenning, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[BenHenning]

Note that I plan to push more fixes to this branch, and additional tests. I'll re-request a review at that time.

[BenHenning]

Note that any additional changes needed in this PR will be covered in #4345 as a fast-follow.

Furthermore, @rt4914's reviews are being deferred (#4266 are tracking each, respectively) as discussed with him previously.

[rt4914]

LGTM Thanks.