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

MV3: Support CSL import from all .csl gitee pages #507

Merged
merged 3 commits into from
Aug 21, 2024

Conversation

jiaojiaodubai
Copy link
Contributor

  1. Github is often inaccessible in Chinese Mainland due to unaudited political content.
  2. In China, Gitee, also based on Git, is widely used as an alternative to GitHub.

Most East Asian style rely on CSL-M to achieve, and these styles are not allowed to be merged into CSL Syle's repository, though some are popular. We have maintained a large number of such styles and synchronized them to Gitee. In order to facilitate their distribution (as well as other styles that may be hosted on Gitee), I made this request.

  • Dev Evn: WSL Ubuntu 22
  • Tested on: Microsoft Edge

let hosts = Zotero.Prefs.get('allowedCSLExtensionHosts');
if (Array.isArray(hosts) && hosts.includes(URI.hostname)) {
if (Array.isArray(hosts) && hosts.some(host => new RegExp(host).test(url))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using regular expressions to test URLs is more compatible, but some variable names may need to be renamed

@adomasven
Copy link
Member

I'm not sure you understand entirely what we're doing with this code.

For Firefox, Safari and Edge with the current Zotero Connector, all you need to do is go into the Zotero Connector Preferences -> Advanced -> Config Editor and change the allowedCSLExtensionHosts parameter to ["raw.githubusercontent.com", "gitee.com"].

MV3 stuff is only relevant with Chrome. We're going to add direct support for allowedCSLExtensionHosts in MV3 though #508

@adomasven adomasven closed this Aug 21, 2024
@jiaojiaodubai
Copy link
Contributor Author

I'm not sure you understand entirely what we're doing with this code.

To my knowledge, MV3 is a new standard for Chrome extensions and has been specially treated in the Connector.

For Firefox, Safari and Edge with the current Zotero Connector, all you need to do is ...

I do know that above settings will work, but as a styles distributor, I cannot guarantee that my users will be able to configure them properly. That's why I am trying to modify upstream code.

@adomasven
Copy link
Member

I've added gitee.com as an allowed csl extension host in 6afc86b

For MV3 (Chrome) support your changes are wrong and won't work. The DNR rule needs to redirect to a different URL (or the same url with a hash like #importConfirm added at the end which will trigger the import prompt. However it would be better covered by implementing #508. If you want to take that on - we'll be happy to accept a PR. Do note that you need to test on Chrome the build in manifestv3 folder.

@jiaojiaodubai
Copy link
Contributor Author

I've added gitee.com as an allowed csl extension host in 6afc86b

Thank you for your prompt response. I manually add "gitee.com" to allowedCSLExtensionHosts in Config Editor, but it didn't work. There is a testing page here. Note that unlike GitHub, the blob page and its raw page have the same host.

The DNR rule needs to redirect to a different URL (or the same url with a hash like #importConfirm added at the end which will trigger the import prompt.

After practical operation, I understand your meaning. We need to modify the URL in Connector to trigger confirmation window. I mistakenly thought that Zotero or the webpage had made such changes.

However it would be better covered by implementing #508.

Does this mean that we will automatically add #importConfirm for sites with supported hosts?

If you want to take that on - we'll be happy to accept a PR.

I am not familiar enough with Connector's code, so I may not be able to handle it.

@adomasven
Copy link
Member

If I install the default Zotero Connector into Firefox, add gitee.com to allowed hosts, and navigate to https://gitee.com/redleafnew00/Chinese-STD-GB-T-7714-related-csl/raw/main/src/gb-t-7714-2015-note-bilingual/gb-t-7714-2015-note-bilingual.csl I get an import prompt.

I have not tested with Edge. Note that if you are building Zotero Connector yourself and testing with Edge, you need to load the extension from the build/chrome folder, not build/manifestv3. We do not distribute the MV3 extension for Edge.

@jiaojiaodubai
Copy link
Contributor Author

jiaojiaodubai commented Aug 21, 2024

OK, I conducted some tests.

  • For Edge and Firefox, simply adding gitee.com in allowedCLSElongationHosts is effective, but if manually filled in, it will require restarting browser or Connector (I speculate that these modifications may only be loaded during some initialization processes).
  • For Chrome, if I add # importConfirm in the code, it will take effect.

But as I mentioned earlier, Gitee's blob page and raw page have the same host, so import prompt is also shown on blob page, which is different from GitHub. Is this tolerable?

@adomasven
Copy link
Member

But as I mentioned earlier, Gitee's blob page and raw page have the same host, so import prompt is also shown on blob page, which is different from GitHub. Is this tolerable?

It's not ideal but acceptable. If you want to modify this PR to make it work properly for MV3, we will accept it along with the regex changes for allowedCSLExtensionHosts.

@adomasven adomasven reopened this Aug 21, 2024
@adomasven adomasven merged commit 2481d09 into zotero:master Aug 21, 2024
@jiaojiaodubai
Copy link
Contributor Author

If you want to modify this PR to make it work properly for MV3, we will accept it along with the regex changes for allowedCSLExtensionHosts.

Yes, of course I hope that users in MV3 environment can also easily access styles on Gitee.

I built the extension based on current PR and tested it on Edge (127.0.2651.105), Firefox (115.14.0esr), and Chrome (127.0.6533.120). It performed well on both Github and Gitee (Firefox refused to allow me to access Github through a proxy network, so it was not tested).

If regular expressions are used to detect websites, then allowedCLSElongationHosts and their context codes may need to be modified to allowedCLSElongationURL or something in order to more clearly express our intention. In addition, I tend to use [^/]+ instead of [^/]* to represent specific URL parts in my code. The details of these code styles are up to you to decide.

@adomasven
Copy link
Member

Merged. Thanks!

We'll keep the allowedCSLExtensionHosts pref name as is. Renaming it requires a pref migration, which is not worth it this change.

northword added a commit to zotero-chinese/website that referenced this pull request Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants