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

Avoid repository settings bug caused by stop-pjax-loading-with-esc #3585

Merged
merged 2 commits into from
Sep 21, 2020

Conversation

artusm
Copy link
Contributor

@artusm artusm commented Sep 21, 2020

  1. LINKED ISSUES:
    Closes Edit button doesn't work on repo settings->Webhooks page when using RGH #3584

  2. TEST URLS:
    https://github.com/sindresorhus/refined-github/settings/hooks
    https://github.com/REPO_OWNER/REPO_YOU_HAVE_ADMIN_PERMS_ON/settings/hooks

  3. SCREENSHOT:
    Add a screenshot here if your PR makes visual changes

@fregante fregante added the bug label Sep 21, 2020
@fregante
Copy link
Member

It feels like this shouldn't pjaxErrorHandler shouldn't be called at all if the user didn't press esc

I don't remember, why is that needed? What triggers it?

I think the solution is to only add the pjax:error if necessary, once, for example inside keydownHandler instead of in init:

window.addEventListener('pjax:error', pjaxErrorHandler, {once: true});

@fregante fregante changed the title fix stop-pjax-loading-with-esc Avoid repository settings bug caused by stop-pjax-loading-with-esc Sep 21, 2020
Co-authored-by: Federico <me@fregante.com>
@artusm
Copy link
Contributor Author

artusm commented Sep 21, 2020

@fregante
Copy link
Member

Indeed, but why would it throw? We're just calling history.back()
Does that cause the error?

@artusm
Copy link
Contributor Author

artusm commented Sep 21, 2020

We're just calling history.back()

which triggers popstate event.

window.addEventListener("popstate", function (t) {
        ut || null == currentPjaxAbortController || currentPjaxAbortController.abort();

which in turn will trigger AbortController.abort().

try {
	const { signal } = (currentPjaxAbortController = new AbortController());
	await fetch(url, { signal } );
} catch (k) {           
    const isPrevented = at(r, "pjax:error");

    if (!isPrevented) {         
       rt && K(null, "", rt.url), window.location.replace(t);
	}
}

Does that cause the error?

When currentPjaxAbortController.abort() is called, the fetch() promise rejects with a DOMException named AbortError.

@fregante fregante merged commit 835b0a0 into refined-github:master Sep 21, 2020
@fregante
Copy link
Member

excellent - YEe4Y10PuOTYI

@Jackenmen
Copy link
Contributor

Thanks for the quick fix and review ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Edit button doesn't work on repo settings->Webhooks page when using RGH
3 participants