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

Linker might run multiple times #55

Closed
SidneyNemzer opened this issue Sep 20, 2019 · 4 comments · Fixed by #56
Closed

Linker might run multiple times #55

SidneyNemzer opened this issue Sep 20, 2019 · 4 comments · Fixed by #56

Comments

@SidneyNemzer
Copy link
Contributor

It looks like the 'don't run more than once' protection is broken because Github changed their markup?

This line injects an element to stop the content script from running multiple times but I think it doesn't work because the element it injects into does not exist (or maybe it doesn't always exist).

In this screenshot, notice that there are multiple dots next to the imports, and the <span id="module-linker-done"> doesn't exist in the page.

image

@fiatjaf
Copy link
Owner

fiatjaf commented Sep 20, 2019

No. I think it was always that way, unrelated to GitHub changes. I didn't know it didn't exist, however. I thought it was a race condition that only happened in Firefox, but apparently it isn't.

Can you conjure up a fix for this?

@SidneyNemzer
Copy link
Contributor Author

Oh interesting. If I have some time this weekend I'll debug a bit more and see what I come up with.

@SidneyNemzer
Copy link
Contributor Author

So after a bit of investigation, it looks like the js-repo-pjax-container ID is not always added to the main. When navigating directly to a file, Github does not add the ID, but navigating to a folder or the root of the repo will get the ID.

Test this for yourself using curl https://github.com/fiatjaf/module-linker | grep main and curl https://github.com/fiatjaf/module-linker/blob/master/package.json | grep main. The first should show a main with the js-repo-pjax-container ID, the second does not.

Normally, this is fine for module linker because you can't navigate directly between files without going to a folder. But I'm using Octotree, which can use some kind of PJAX request from any page. If I go directly to a file, then use Octotree to navigate to another, module-linker links things more than once because the ID is missing.

This is pretty easy to fix, by replacing everything selecting #js-repo-pjax-container with main. In my brief testing, this fixed the problem and did not cause any other issues. (It also allows module-linker to PJAX request when you navigate directly to a file and click a linked module from the same repo, which it cannot right now).

I can open a PR if you think this is a good fix!

@fiatjaf
Copy link
Owner

fiatjaf commented Sep 23, 2019

I think it is a good idea and you've solved the biggest issue this project has ever faced. Please open a PR.

SidneyNemzer added a commit to SidneyNemzer/module-linker that referenced this issue Sep 24, 2019
It seems that Github does not always add the ID to `main`, but the `main` is always there.

Fixes fiatjaf#55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants