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

Bugfix: Synctex does not work while using vscode in browser #4220

Merged
merged 3 commits into from
Apr 3, 2024
Merged

Bugfix: Synctex does not work while using vscode in browser #4220

merged 3 commits into from
Apr 3, 2024

Conversation

wasalm
Copy link
Contributor

@wasalm wasalm commented Apr 2, 2024

Although code-server is not officially supported, hereby a bugfix making (ctrl) + click work in the browser.

Namely, before synctex is enabled, it waits before PDF.js is loaded. PDF.js signals this by sending a custom "webviewloaded" event. It first tries to send it to the parent window and if this fails, it send it to its own window.
in code-server, this parent exists as a iframe, hence this event is where there are no listeners. Issue is fixed by added a listener to the parent window if possible.

wasalm added 2 commits April 2, 2024 21:09
Make synctex work in the code server environment
@James-Yu
Copy link
Owner

James-Yu commented Apr 3, 2024

This patch will add two listeners to the event if the webpage is not in code-server. parent.document exists even in such environments.

@wasalm
Copy link
Contributor Author

wasalm commented Apr 3, 2024

Agree, it will create two listeners. However, I don't think this will create an issue because

  1. the event is only fired once in viewer.mjs between the lines 14884 and 14896. It is either fired in parent.document or document itself.
  2. in the hypothetical case it would fire multiple times, the promise webViewerLoaded in latexworkshop.ts will only be resolved once, because any second call to resolve() will be ignored.
  3. This event will only be called once, and hence this extra overhead will not be significant.

@James-Yu
Copy link
Owner

James-Yu commented Apr 3, 2024

What will be the value of document in a code-server env? If undefined, document ?? parent.document should work well.

@wasalm
Copy link
Contributor Author

wasalm commented Apr 3, 2024

Also it is good to understand why the origional code does work in vscode. The event "webviewerloaded" is only created in viewer.mjs. Here it tries to dispatch this event first in parent.document and if it fails it dispatches it in document itself.

The listener for the "webviewerloaded" event is in the file latexworkshop.ts. It shares the same document with viewer.mjs, because viewer.mjs gets loaded by latexworkshop.ts at line 824. Hence in the origional version, the code will only work if it gets dispatched in document and not in parent.document.

For the code-server the parent document and document are on the same origin and hence it is allowed to dispatch events in the parents. I expect that in vscode this will be different and hence when it tries to dispatch in parent, it will give an error.

@wasalm
Copy link
Contributor Author

wasalm commented Apr 3, 2024

What will be the value of document in a code-server env? If undefined, document ?? parent.document should work well.

document and parent.document are both well-defined objects, so document ?? parent.document will not work. the root issue is that pdf.js might send the "webviewerloaded" event to a different iframe than latexworkshop.ts is expecting

@James-Yu James-Yu merged commit 88c83b2 into James-Yu:master Apr 3, 2024
0 of 6 checks passed
James-Yu added a commit that referenced this pull request Apr 10, 2024
* Bugfix syntex for code-server

Make synctex work in the code server environment

* Update latexworkshop.ts

typo

* Update latexworkshop.ts

---------

Co-authored-by: James Yu <[email protected]>
James-Yu added a commit that referenced this pull request Apr 16, 2024
commit eca259d
Author: James Yu <[email protected]>
Date:   Mon Apr 15 17:52:06 2024 +0100

    Fix #4233 Cache outdir/auxdir per root file

commit 729fe8a
Author: James Yu <[email protected]>
Date:   Mon Apr 15 13:41:04 2024 +0100

    Fix #4215 Add `tkz-euclide` suggestions, update some others

commit 8abc746
Author: James Yu <[email protected]>
Date:   Mon Apr 15 12:53:34 2024 +0100

    Fix a typing issue

commit a7aa1fb
Author: James Yu <[email protected]>
Date:   Mon Apr 15 12:48:22 2024 +0100

    Version 9.20.0

commit 5e15cab
Author: James Yu <[email protected]>
Date:   Mon Apr 15 12:43:53 2024 +0100

    Prioritize SyncTeX binary over synctex.js

commit 2163ae2
Author: Ken Peng <[email protected]>
Date:   Mon Apr 15 18:44:32 2024 +0800

    Fix forward SyncTex accuracy in internal browser by providing a range location indication alternative (#4194)

    * Add forward SyncTex with range location indication.

    * Remain default behavior of using red circle for forward SyncTex.

    * Remove unused column parameter in callSyncTeXToPDFRange.

    * Merged configuration `latex-workshop.synctex.indicator.type` to `latex-workshop.synctex.indicator.enabled`.

    Added fallback logic in `toPDF` for boolean type of `latex-workshop.synctex.indicator.enabled`.

    Extend type `SyncTeXRecordToPDFAll` from base type `SyncTeXRecordToPDF`.

    Merged `locateRange` to `locate`.

    Remove `SyncTeXRecordToPDFAllList`.

    Merged `callSyncTeXToPDFRange` to `callSyncTeXToPDF`.

    In `ServerResponse`, added and extended type `SynctexData` from base type `SynctexRangeData`.

    * Merged `forwardSynctexRange` with `forwardSynctex` in viewer.

    * Indent fixing.

    * Change the enum name.

    Simplify if-else clauses.

    Change rectangle default color.

    Add comments.

    * Code style tweaks

    ---------

    Co-authored-by: James Yu <[email protected]>

commit 68c5e74
Author: James Yu <[email protected]>
Date:   Mon Apr 15 10:55:58 2024 +0100

    Fix #4215 Use `kpsewhich.class.enabled` and `kpsewhich.bibtex.enabled` to control `kpsewhich`

commit afe93c7
Author: James Yu <[email protected]>
Date:   Fri Apr 12 11:03:53 2024 +0100

    Fix #4227 Ignore label defs in `xparse` macros

commit 16b504e
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Thu Apr 11 14:50:40 2024 +0100

    Bump tar from 6.2.0 to 6.2.1 (#4226)

    Bumps [tar](https://github.com/isaacs/node-tar) from 6.2.0 to 6.2.1.
    - [Release notes](https://github.com/isaacs/node-tar/releases)
    - [Changelog](https://github.com/isaacs/node-tar/blob/main/CHANGELOG.md)
    - [Commits](isaacs/node-tar@v6.2.0...v6.2.1)

    ---
    updated-dependencies:
    - dependency-name: tar
      dependency-type: indirect
    ...

    Signed-off-by: dependabot[bot] <[email protected]>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

commit e2a5b60
Author: Jerome Lelong <[email protected]>
Date:   Thu Apr 11 08:10:26 2024 +0200

    Update grammar files to jlelong/vscode-latex-basics@0393049

commit 7773d50
Author: Jerome Lelong <[email protected]>
Date:   Tue Apr 9 07:18:58 2024 +0200

    Update grammar files to jlelong/vscode-latex-basics@6a16810

commit 0738417
Author: James Yu <[email protected]>
Date:   Thu Apr 4 13:09:40 2024 +0100

    Version 9.19.2

commit 0483b31
Author: James Yu <[email protected]>
Date:   Thu Apr 4 13:08:14 2024 +0100

    Prevent `.aux` and `.out` file changes from triggering auto-build

commit 88c83b2
Author: wasalm <[email protected]>
Date:   Wed Apr 3 15:21:55 2024 +0200

    Bugfix: Synctex does not work while using vscode in browser (#4220)

    * Bugfix syntex for code-server

    Make synctex work in the code server environment

    * Update latexworkshop.ts

    typo

    * Update latexworkshop.ts

    ---------

    Co-authored-by: James Yu <[email protected]>

commit 14fef41
Author: Jerome Lelong <[email protected]>
Date:   Tue Mar 26 21:47:07 2024 +0100

    Update grammar files to jlelong/vscode-latex-basics@70d2764
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants