Refined-github: Stop ajax loading with `esc`

[fregante]

Current

1. Click on the "Issues" tab above
2. Change your mind
3. Can't do anything, Issues keeps loading
4. When loaded, go back

Desired

1. Click on the "Issues" tab above
2. Change your mind
3. Press esc, loading stops

⬆️ this is how regular pages work, but pjax doesn't listen to esc

---

IssueHunt Summary

artusm has been rewarded.

Backers (Total: $40.00)

• issuehunt ($40.00)

Submitted pull Requests

- #3465 Add stop-pjax-loading-with-esc feature

Tips

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on IssueHunt to raise funds.
  

[sindresorhus]

Sounds good.

[issuehunt-app[bot]]

@issuehunt has funded $40.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

[fregante]

pjax doesn't seem to offer a Cancel/Abort action.

To implement this you'd have to come up with some hack. Hope someone can think of something.

[jerone]

It looks like the pjax on GitHub doesn't return the xhr with pjax:beforeSend.

$(document).on("pjax:beforeSend",function(){console.log(this, arguments);});
$("#js-repo-pjax-container").on("pjax:beforeSend",function(){console.log(this, arguments);});
document.getElementById("js-repo-pjax-container").addEventListener("pjax:beforeSend",function(){console.log(this, arguments);});
document.addEventListener("pjax:beforeSend",function(){console.log(this, arguments);});

[notlmn]

I'm not sure if https://github.com/defunkt/jquery-pjax is the one GitHub is using, because most of the events are missing from GitHub's JS files.

So, I came up with a hack using AbortController, that involves replacing fetch at global level.

const actualFetch = self.fetch; // Saving original fetch for later use
const controller = new AbortController();

// Custom fetch, replacing `AbortSignal` on request object
self.fetch = async (request) => {
    console.log(request);
    const newRequest = new Request(request, {signal: controller.signal}); // Create new request, with our own signal
    try {
        return actualFetch(newRequest);
    } catch (error) {
        console.error(error);
    }
};

// Handler to abort fetch
window.addEventListener('keydown', (event) => {
    if (event.key === 'Escape') {
        console.log('Aborting...');
        controller.abort();
    }
});

One catch with the above snippet is that after the fetch is aborted, the user is directly redirected to the target page, which is no the intended behavior, we need the user to stay on the current page.

I think we can manage this using as very clever combination of mousedown, click, and mouseup event handlers and using stopPropagation() if the click actually caused that aborted fetch (haven't tested that part though).

[fregante]

replacing fetch at global level

Make sure that this actually works because last time I tried to do that in the content script it failed in Firefox, but now I'm testing in the console and it appears to work:

When testing, keep in mind that this replacing code has to be injected _unsafely_ in the main page like resolve-conflicts because content scripts are in a different realm.

[fregante]

Additionally, I think that GitHub will actually try to reload the page when the fetch fails, so esc would have to be pressed twice: once to stop the fetch and again to stop the reload.

[TheGuardianWolf]

Hey @fregante is this still an active issue?

[fregante]

It’s still active, but I don’t think it’s solvable. Do you want to try?

[kidonng]

Has anyone sent feedback to GitHub? If not I'm going to make one 😁

[TheGuardianWolf]

I can try, but in my firefox and chrome, I'm able to ESC out of loading issues, but only issues, is this the same for you guys?

[fregante]

I'm able to ESC out of loading issues, but only issues

I think that's the regular ESC feature of the browser because they are not ajaxed pages. That's what this issue is about: ajaxed pages don't support ESC

[fregante]

Has anyone sent feedback to GitHub? If not I'm going to make one 😁

Please do!

[TheGuardianWolf]

@fregante

One hack coming right up, we can do this with webextensions like the following:

const ajaxRequests = new Set<string>();
const cancelRequests = new Set<string>()

browser.commands.onCommand.addListener((command) => {
    if (command === 'stop-request') {
        cancelRequests.clear();
        Array.from(ajaxRequests).forEach((ajaxUrl) => {
            const requestedUrl = ajaxUrl.match(/^(.*)[?&]_pjax=.*$/i)[1];
            if (requestedUrl) {
                cancelRequests.add(ajaxUrl);
                cancelRequests.add(requestedUrl);
            }
        })
    }
})

browser.webRequest.onBeforeRequest.addListener((details) => {
    ajaxRequests.add(details.url);
}, { urls: ['https://github.com/*?*_pjax=*'], types: ['xmlhttprequest'] });

browser.webRequest.onHeadersReceived.addListener((details) => {
    ajaxRequests.delete(details.url);
}, { urls: ['https://github.com/*?*_pjax=*'], types: ['xmlhttprequest'] });

browser.webRequest.onHeadersReceived.addListener((details) => {
    if (cancelRequests.has(details.url)) {
        cancelRequests.delete(details.url);
        return { cancel: true };
    }
}, { urls: ['https://github.com/*'], types: ['main_frame', 'xmlhttprequest'] }, ['blocking']);

How this works is such that all ajax requests are tracked between the request being made and the headers being recieved.

When an interrupt is called, all requests that are tracked will be canceled.

This particular example is a background script only example, to cancel on esc involves making a content script that communicates with this background script as background scripts are limited in what keys they can intercept.

The gotcha here is that pjax becomes confused and some html content seems to lose their formatting. I've tried a redirect on ajax requests instead and this works (code is below), but content is rerendered (even through the page does not switch).

browser.webRequest.onHeadersReceived.addListener((details) => {
    if (cancelRequests.has(details.url)) {
        cancelRequests.delete(details.url);
        if (details.type === 'main_frame') {
            return { cancel: true };
        }
        else if (details.type === 'xmlhttprequest') {
            return { redirectUrl: details.originUrl };
        }
    }
}, { urls: ['https://github.com/*'], types: ['main_frame', 'xmlhttprequest'] }, ['blocking']);

I also tried out @notlmn 's solution, which seems to have weird behaviour, but it triggers the same rendering bug as mine. Additionally, I think if I fuse our solutions we could have something that doesn't need to track ajax requests if you prefer it that way.

[notlmn]

I don't believe we have access to browser.webRequest, if I'm not wrong isn't it a new permission we need to ask the user for?

[TheGuardianWolf]

Yeah, there are two permissions needed here webRequest and webRequestBlocking

[notlmn]

Yeah, there are two permissions needed here webRequest and webRequestBlocking

Yeah, we wouldn't really want to add more permissions if we could do it any other way.

Also how is cancelling requests using browser.webRequest different from signalling using AbortController?

[TheGuardianWolf]

It's not different in cancellation, but the other method of the self redirect is possible through webRequests only.

The main advantage is not in cancelling the ajax request, as you've already shown that to be possible, but in cancelling the page load afterwards that pjax falls back on.

Was that the reason why you didn't continue with your fix? Or was there some other complication I'm not aware of?

I'm confused as to why this might be unsolvable given there are solutions to cancel requests, it's just whether or not they fit the scope of this plugin.

[fregante]

Yeah, there are two permissions needed here webRequest and webRequestBlocking

We can't add those 2 permissions by default, which means we'd have to have a UI for this specific feature to request them (browser.permission.request). For example it could append a message like

ESC won't cancel ajaxed loads, but `xyz-feature` can if you add extra permissions: [Add]

If your solution works, feel free to send a PR, keeping in mind that 👇

some html content seems to lose their formatting
content is rerendered

👆 these are probably deal-breakers. The point of this feature would be to avoid changing page; For example if you're not down leaving a review comment, a rerendering would still lose the comment.

[TheGuardianWolf]

That makes sense, then the cancellation method is more in line with what you want.

With that method, what I've seen is two issues:

• The profile page's username is not rendered properly after cancelling load on the profile page (if you scroll so it's not visible then scroll back, it corrects itself)
• DevTools no longer shows HTML after cancelling load

If these two issues are not acceptable then I guess it really can't be approached from outside of github code, since both my method and the fetch replacement produce these errors

[fregante]

• The profile page's username is not rendered properly after cancelling load on the profile page

This might be due to some _unload_ handler that we cannot prevent. We can consider these separately, like if it breaks profiles, we can disable it there, as long as it doesn't break _almost everywhere_

• DevTools no longer shows HTML after cancelling load

Not a problem, GitHub users generally don't need to open the devtools.

Perhaps you can send a minimal PR to test it out

[TheGuardianWolf]

Wow, that pull request by @artusm is an elegant solution. I think that is your best bet.

To think that pjax event hooks were right under our noses 😆

[fregante]

From "impossible" to "done"

Thanks @artusm!

[issuehunt-app[bot]]

@sindresorhus has rewarded $36.00 to @artusm. See it on IssueHunt

• :moneybag: Total deposit: $40.00
• :tada: Repository reward(0%): $0.00
• :wrench: Service fee(10%): $4.00