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

Can't open repository page #39

Closed
An-dz opened this issue May 24, 2021 · 17 comments
Closed

Can't open repository page #39

An-dz opened this issue May 24, 2021 · 17 comments
Labels
bug Something isn't working

Comments

@An-dz
Copy link
Contributor

An-dz commented May 24, 2021

Description

The repository page does not want to open with 4.3.1, had to revert to 4.3.0. Error log is below.

Everything else works.

Log

Uncaught (in promise) TypeError: Cannot read property 'whenDefined' of null
    at Object.<anonymous> (github-defreshed.user.js:16)
    at Generator.next (<anonymous>)
    at a (github-defreshed.user.js:16)

Other information

  • Link to page with the issue: https://github.com/Kir-Antipov/GitHub-Defreshed
  • Browser: Vivaldi w/ Chromium 91.0.4472.66 & Woolyss Ungoogled Chromium 90.0.4430.212
  • Operating System: Windows 10 x64 20H2
  • GitHub-Defreshed version: 4.3.1 user-script version
@An-dz An-dz added the bug Something isn't working label May 24, 2021
@Kir-Antipov
Copy link
Owner

Kir-Antipov commented May 24, 2021

Well, it seems that whenDefined isn't supported by your browser. Wait a moment, please, will fix it in a minute

@Kir-Antipov
Copy link
Owner

I hope v4.3.2 is okay :)

@Kir-Antipov
Copy link
Owner

Note: in fact, custom elements are supported by the browser, but access to this functionality from content scripts is limited (as far as I can say by this issue)

@An-dz
Copy link
Contributor Author

An-dz commented May 24, 2021

Still broken. But now there's no error on console, the loading just keeps spinning forever.

Checking the HTML it's normal crappy GitHub.

@Kir-Antipov Kir-Antipov reopened this May 24, 2021
@Kir-Antipov
Copy link
Owner

Hm. It's really strange. I'm very sorry for the inconvenience, tomorrow I'll try to find a solution to this problem

@Kir-Antipov
Copy link
Owner

Hi! Didn't issue magically resolve itself while I was asleep? :D

Cannot reproduce the issue with Ungoogled Chromium 90.0.4430.212
image

@Kir-Antipov
Copy link
Owner

Kir-Antipov commented May 25, 2021

If the problem persists, could you run this snippet on the repository page while it refuses to load, please?

const refSelector = document.querySelector("#branch-select-menu ref-selector");
console.log(refSelector.constructor);
await refSelector.index.fetchData();
console.log(refSelector.index.currentSearchResult);

Expected output:
image


If that snippet did work, could you check if content scripts in your browser have access to RefSelectorElement? This may help you:

// ==UserScript==
// @name         RefSelectorElement test
// @namespace    GitHub
// @version      0.1
// @match        https://github.com/*
// @run-at       document-idle
// @grant        none
// ==/UserScript==
(function() {
    "use strict";
    alert(window.RefSelectorElement ? "RefSelectorElement is accessible" : "RefSelectorElement is not accessible");
})();

@An-dz
Copy link
Contributor Author

An-dz commented May 25, 2021

The output is the same as your image and putting RefSelectorElement on the content script says it's not available, as I expected since content scripts can't access the page variables as it runs on a separate layer.

The same problem is happening on Firefox 78.

@Kir-Antipov
Copy link
Owner

Content scripts can access and modify the page's DOM, just like normal page scripts can. They can also see any changes that were made to the DOM by page scripts.
However, content scripts get a "clean" view of the DOM. This means:

  • Content scripts cannot see JavaScript variables defined by page scripts.
  • If a page script redefines a built-in DOM property, the content script sees the original version of the property, not the redefined version.

Oh. Now I see. But I can't understand 2 things:

  1. Why is this even a thing? "Clean view of the DOM". Benefits of this are highly questionable
  2. Why it doesn't work for me (-> GitHub-Defreshed doesn't fail)? :D

@An-dz
Copy link
Contributor Author

An-dz commented May 25, 2021

Why is this even a thing? "Clean view of the DOM". Benefits of this are highly questionable

Yes, not "write" to it seems reasonable so you don't need to worry about your variables breaking the page, but reading is pretty dumb.

Why it doesn't work for me (-> GitHub-Defreshed doesn't fail)? :D

At first I thought it could be you using Firefox, but it fails on mine and you have tested on Chromium.

But maybe it's how we have installed it. Are you using your XPI for Firefox? Or GreaseMonkey?

Kir-Antipov added a commit that referenced this issue May 25, 2021
@Kir-Antipov
Copy link
Owner

Yes, not "write" to it seems reasonable so you don't need to worry about your variables breaking the page, but reading is pretty dumb

Completely agree! "This means that content scripts can rely on DOM properties behaving predictably, without worrying about its variables clashing with variables from the page script." Guys, "DOM properties behave predictably" when and only when they work same way they do when I'm browsing the website I made the script for...


But maybe it's how we have installed it. Are you using your XPI for Firefox? Or GreaseMonkey?

Ahaha, I forgot about GreaseMonkey :D I use TamperMonkey (yep, I know it's closed-source and so on, but I'm just used to it) and it seems it doesn't use Content Script API :D

@Kir-Antipov
Copy link
Owner

Well, I hope I've fixed the issue (+1 api call due to Content Script API restrictions -_-). Could you please check if this fix works for you, so I won't release broken version again? :D

You can grab build artifacts here

@An-dz
Copy link
Contributor Author

An-dz commented May 26, 2021

Ahaha, I forgot about GreaseMonkey :D I use TamperMonkey (yep, I know it's closed-source and so on, but I'm just used to it) and it seems it doesn't use Content Script API :D

I don't use GreaseMonkey either, I used to use ViolentMonkey, but now I just create a pseudo-extension with a manifest and load from disk. For Firefox I use your XPI.

Well, I hope I've fixed the issue (+1 api call due to Content Script API restrictions --)_. Could you please check if this fix works for you, so I won't release broken version again? :D

It works now. 👍

@An-dz An-dz closed this as completed May 26, 2021
@Kir-Antipov
Copy link
Owner

I don't use GreaseMonkey either, I used to use ViolentMonkey, but now I just create a pseudo-extension with a manifest and load from disk. For Firefox I use your XPI

Isn't it a little bit tedious to do all of these manually? Why did you stop using ViolentMonkey?

It works now. 👍

Finally, haha :) I'll make few more changes (aka fixes, e.g., busy status is broken again) and then will release new version

Thank you so much for helping out with this one! I'm always too lazy to check that everything works outside of my usage scope (Firefox + userscript), so Chrome/Web Extension bugs are unnoticeable for me :D

@Kir-Antipov
Copy link
Owner

v4.3.3 (which fixes the issue) was published :)

@An-dz
Copy link
Contributor Author

An-dz commented May 26, 2021

Isn't it a little bit tedious to do all of these manually?

I just need to create the manifest once and then I just download the .user.js over the old one whenever you update it. GitHub alerts me when a release is made.

Why did you stop using ViolentMonkey?

It runs over every page even though my userscripts only run over some pages, Chromium will handle that faster and more secure for me. And the only userscript I use that I have not created is yours so I don't care about manually updating it.

@Kir-Antipov
Copy link
Owner

Oh, I see, that's reasonable :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants