Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix setFullScreenMode on talk viewer opening #1331

Merged
merged 1 commit into from
Aug 24, 2022
Merged

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Aug 17, 2022

Because Talk is doing some shenanigans to fake the Files Sidebar

Uncaught (in promise) TypeError: OCA.Files.Sidebar.setFullScreenMode is not a function
    at viewer-main.js:2
    at u (talk-main.js:1)
    at Generator._invoke (talk-main.js:1)
    at Generator.next (talk-main.js:1)
    at Rw (viewer-main.js:2)
    at a (viewer-main.js:2)
    at viewer-main.js:2
    at new Promise (<anonymous>)
    at viewer-main.js:2
    at a.openFile (viewer-main.js:2)

@skjnldsv skjnldsv added bug Something isn't working 3. to review Waiting for reviews labels Aug 17, 2022
@skjnldsv skjnldsv self-assigned this Aug 17, 2022
@skjnldsv
Copy link
Member Author

/backport to stable24

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Aug 17, 2022
@skjnldsv
Copy link
Member Author

/rebase

@skjnldsv
Copy link
Member Author

/compile amend /

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have reproduction steps? I have never seen that error (and I also tried to reproduce it without success), and it should not happen, as the fake sidebar has a setFullScreenMode function.

If the issue is caused by Talk I think it would be better to fix it in Talk instead (if it is possible, which I do not know :-) ).

@skjnldsv
Copy link
Member Author

Maybe I was too much behind lol
In any case, this is good sanity check ;)

@skjnldsv skjnldsv requested a review from danxuliu August 19, 2022 07:57
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I was too much behind lol

More than a year, it is surprising that Talk even loaded :-O

In any case, this is good sanity check ;)

Then typeof OCA?.Files?.Sidebar?.setFullScreenMode === 'function'? Or that is too much? :-P

Anyway, fine by me to keep the old code, to check if setFullScreenMode exists or to check if setFullScreenMode is a function 👍

@skjnldsv skjnldsv closed this Aug 19, 2022
@skjnldsv skjnldsv deleted the fix/sidebar-talk-open branch August 19, 2022 10:47
@skjnldsv skjnldsv restored the fix/sidebar-talk-open branch August 19, 2022 10:47
@skjnldsv skjnldsv reopened this Aug 19, 2022
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 24, 2022
@skjnldsv skjnldsv merged commit fcd357d into master Aug 24, 2022
@skjnldsv skjnldsv deleted the fix/sidebar-talk-open branch August 24, 2022 14:29
@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish backport-request Pending backport by the backport-bot bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants