fix: save scroll position on exit from video xblock fullscreen mode

[ihor-romaniuk]

This merge request contains a fix for toggling video xblock full-screen mode and saving the previous window top offset position on exit from the full-screen state.

A related bug was found here https://bugs.chromium.org/p/chromium/issues/detail?id=142427 but it still reproduces.

Realised solution: Save the scroll position before the turn on the fullscreen mode and scroll to the previous position on turn off the fullscreen mode.

Dependent PR to MFE Learning: This MR https://github.com/openedx/frontend-app-learning/pull/982 must be merged with this MR.

[openedx-webhooks]

Thanks for the pull request, @ihor-romaniuk! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[mphilbrick211]

Hi @ihor-romaniuk! Just checking in on this to see if you are planning to pursue, and re-run the failing tests?

[ihor-romaniuk]

Hi @mphilbrick211. I'm not sure that my changes have affected some python code and their checkers. We have errors with unsuccess installing python packages.

[mphilbrick211]

Hi @ProductRyan! Have you had a chance to review this?

[mphilbrick211]

@jmakowski1123 @ProductRyan looks like the roadmap ticket is closed - is product review done?

[mphilbrick211]

@jmakowski1123 @ProductRyan looks like the roadmap ticket is closed - is product review done?

Nevermind - I see it's listed under https://github.com/openedx/platform-roadmap/issues/229

[mphilbrick211]

Hi @ihor-romaniuk - would you mind taking a look at the failing tests? Thanks!

---

General Note: 4/5/23 - Product review complete.

@openedx/content-aurora @mattcarter - is this something you can take a look at for review / merge? This PR is related to the other "scroll position" PRs listed here: #31053.

[ihor-romaniuk]

Hi @mphilbrick211. I'm not sure that I need to fix this failed tests in my PR because it doesn't connect to my changes.

[mphilbrick211]

Hi @ihor-romaniuk - I think all checks need to be green regardless in order to merge.

@openedx/content-aurora @mattcarter - Hi there! Just seeing if you are able to review/merge this for us? Thanks!

[mphilbrick211]

Hi @ihor-romaniuk! Just checking to see if you will be re-running the failing tests?

[mphilbrick211]

Hi @ihor-romaniuk! Just checking to see if you will be re-running the failing tests?

Hi @ihor-romaniuk - checking back in on this.

[ihor-romaniuk]

Hi @mphilbrick211 I have no idea why python checkers fall because I do nothing with this code. I have tried to rerun them, but the result was the same. Maybe someone can help us with it?

[mphilbrick211]

Hi @mphilbrick211 I have no idea why python checkers fall because I do nothing with this code. I have tried to rerun them, but the result was the same. Maybe someone can help us with it?

@e0d - would you be able to shed some light on this?

[itsjeyd]

@ihor-romaniuk If the target branch (openedx:open-release/maple.master) has changed since you last pushed your changes, perhaps you could try rebasing and pushing them again?

CC @mphilbrick211 -- I came here from https://github.com/openedx/frontend-app-learning/pull/982 which @leangseu-edx pointed out should be merged together with this PR.

Also CC @e0d, in case you have additional comments about why tests might be failing here.

[ihor-romaniuk]

I tried to update it again, but the error is still there.

[mphilbrick211]

I tried to update it again, but the error is still there.

@itsjeyd @e0d flagging this.

[itsjeyd]

@mphilbrick211 @ihor-romaniuk I'm not sure what's going on with the first check (CI / check migrations) -- there's no command line output to check for specific errors under Details.

However, all the other failing checks seem to be caused by the same issue:

Obtaining file:///runner/_work/edx-platform/edx-platform/common/lib/capa (from -r requirements/edx/testing.txt (line 7))
Obtaining codejail==3.1.3 from git+https://github.com/edx/[email protected]#egg=codejail==3.1.3 (from -r requirements/edx/testing.txt (line 11))
  WARNING: git clone in /edx/app/edxapp/venvs/edxapp/src/codejail exists with URL https://github.com/openedx/codejail.git
  WARNING: The plan is to install the git repository https://github.com/edx/codejail.git
ERROR: Exception:
Traceback (most recent call last):
 ...
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/pip/_internal/utils/misc.py", line 247, in ask
    response = input(message)
EOFError: EOF when reading a line

Something's going wrong when trying to install codejail from https://github.com/edx/codejail.

Based on a brief look, the codejail repo doesn't exist at this URL anymore; https://github.com/edx/codejail redirects to https://github.com/openedx/codejail. Maybe that's why the installation step fails?

@ihor-romaniuk You could try updating requirements to point to the new URL for codejail and see if that helps.

CC @e0d in case you have any additional pointers here.

[ihor-romaniuk]

Closed due to outdated edx version and no need for these changes. Related MR also closed https://github.com/openedx/frontend-app-learning/pull/982.

[openedx-webhooks]

@ihor-romaniuk Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.