-
Notifications
You must be signed in to change notification settings - Fork 167
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
Various Improvements #379
base: master
Are you sure you want to change the base?
Various Improvements #379
Conversation
* Update HTML and CSS files so they format properly on Android (Firefox) * Update popup.js so the popup closes when clicking "Edit Redirects" and Android * Make the example rule use HTTPS as now most browsers upgrade to HTTPS by default
* Change options_ui in manifest so it workss as expected (especially on mobile) * Clean up HTML, CSS & JS files so they're easier to read
* Update importRedirects function in importexport.js so it uses array methods for improved simplicity and readability * Update the rules logic to use a for...of loop for improved simplicity and readability * Use template literals to improve readability
* improve readability and function of importexport.js (modernize, fix redundancy, remove example rule when importing more than 1 rule) * Fix example rule not working on install * More minor CSS consistency updates
@Gitoffthelawn Tested with Brave, Librewolf, and Firefox Nightly on Ubuntu 23.04, 23.10, and NixOS 23.11 as well as Fennec F-Droid v121.0.0 on Android 14. My 43 rules have had zero issues. |
Thank you. Would it be possible for you to test on desktop Firefox ESR as well (currently at v115)? |
Of course, I'll ping you again after I use it for another at least 12 hours between browsers and computers. |
@NyeUsr Thanks. Take your time. I have a busy week scheduled... |
* Import now updates pre-existing rules instead of making duplicates * Minor bug fix on mobile * Update http: links to https: in help.html
Import now updates pre-existing rules instead of making duplicates which should fix #339 . In my testing everything continues to work wonderfully. Currently it checks the description and if a rule with the description matches it updates the rule, anyone see any issue or anything I may have missed with this? @Gitoffthelawn @KaKi87 |
* Duplicate Button now increments numbers in the description to help prevent the possibility of import issues
Sorry but that's definitely bad : what would happen I change a rule's description ? 😅 You must use something that the user cannot change, i.e. an ID. |
Interesting. I hadn't thought about you changing the description of a redirect. Guess I always treated it as a title lol. I'll see what I can figure out. |
It is a title. And a title is user-editable. |
Yeah. The ID would be stored in the JSON file, which is less likely to be edited by the user. Currently, I believe the issue with either solution is that if you choose to import content from another user, which I often do, it could potentially overwrite one of your redirects if they end up having the same ID or name. Although this scenario is relatively unlikely I would prefer to err on the side of caution. As part of my planned update to the import feature in general, which is a larger project requiring more time, I can address this. I intend to add a functionality to handle importing from URLs, making it more convenient to import examples and community rules without the need for manual copy-pasting. Within this new import menu, I can include a checkbox for merging or updating rules based on title and/or ID. What're your thoughts on this approach? |
* Improvements to the UI on mobile * Fix import button not working more than once per page load
That cannot realistically happen with IDs as long as you're using
No, only ID must be used, there is no other choice to be. |
Sorry, should've updated that or made a new post. I've been testing using nanoid since before your post. I honestly don't know why I included 'based on title and/or ID.' either since before I even sent that I was looking to use ID only. As always I appreciate the replies, thank you! |
* New import popup allowing both import from file and import from url * Unique rule IDs based on Nano ID * Rules without an ID will be automatically assigned an ID (for upgrading users and also applies to imports) * Fixed Base64 decode * Replace depreciated unescape code in redirect.js
I've been testing this newest version for the past couple days and it seems to work very well. Anyone else able to test it? |
looks good for me. @Gitoffthelawn are you still maintaining this project? |
Oh I'm sorry I forgot to, I will do shortly ! |
* Update renderRedirects() to reduce DOM manipulations * Minor CSS consistency updates - Been sitting on these for a while now...
Yes, however I don't currently have what I need to release a new version. Let me see if I can arrange to get everything I need. |
I think this PR is really needed for Android. are there any plans to merge it? |
I'm seeing some problems with this one. Some significant problems.
For testing purposes, I made some short lists. One has 15 rules. One has 6 rules, and is a pruned-down version of the other. If I import them both, I get 21 rules. Even when I refresh the page, the duplicates are still there. But it gets worse. If I import again, and I'm not sure exactly how it's happening, but the rules I import can overwrite the rules that are already there. This could result in a loss of user rules. And personally, if that happened to me, it would ruin my day. |
I cannot recreate this. My result of trying to import a list multiple times is that, instead of overwriting existing rules that have a matching unique ID it instead just duplicates them. Can you give more information like Browser and OS?
Yeah, I get
Okay, so, does the pruning down remove the unique ID that's required for the rule to be updated? Or is it fixed by refreshing between each import? If it's the latter, the temporary fix for the second issue you found would fix it.
Is it overwriting rules that have a matching unique ID or unrelated rules? Similar to the first issue, I cannot recreate this. |
Mobile Compatibility and Usability:
Import Functionality Enhancements:
Rule Adjustments:
Code Readability and Efficiency:
Additional Changes:
As of the last update 16/04/2024