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

fix(frontend): get_list_block_sieve function not callable if sievefilters module… #1435

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christer77
Copy link
Member

@christer77 christer77 changed the title fix:get_list_block_sieve function not callable if sievefilters module… fix(sievefilters):get_list_block_sieve function not callable if sievefilters module… Jan 28, 2025
@christer77 christer77 changed the title fix(sievefilters):get_list_block_sieve function not callable if sievefilters module… fix(other): get_list_block_sieve function not callable if sievefilters module… Jan 28, 2025
@@ -977,6 +977,26 @@ function sieveFiltersPageHandler() {
});
}

function get_list_block_sieve() {
sessionStorage.removeItem('list_blocked');
var detail = Hm_Utils.parse_folder_path(hm_list_path());
Copy link
Member

Choose a reason for hiding this comment

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

  1. You should avoid using var at all costs because it can lead to unintended global variable pollution.
  2. hm_list_path() and hm_page_name() are deprecated and do not always guarantee accurate results. Utility functions in modules/core/navigation/utils.js should be used instead.
  3. What happens to the code calling get_list_block_sieve() when the module is disabled?
  4. Did the pull request title miss something, and why does it use the tag other? Shouldn't frontend be appropriate as the tag?

Copy link
Member Author

Choose a reason for hiding this comment

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

The objective is to getthe list of blocked addresses, when the mobule is deactivated, nothing happens, i.e. the function no longer get this list

Copy link
Member

Choose a reason for hiding this comment

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

When the module is deactivated, get_list_block_sieve will be undefined, thus, get_list_block_sieve() will throw an error in the handler function.

@christer77 christer77 force-pushed the get_list_block_sieve_to_sieve_module branch from c1d6a36 to 2551331 Compare February 3, 2025 14:44
@christer77 christer77 changed the title fix(other): get_list_block_sieve function not callable if sievefilters module… fix(frontend): get_list_block_sieve function not callable if sievefilters module… Feb 3, 2025
@mercihabam
Copy link
Member

It looks like some words are being truncated in your pull request title. Make sure to expand the title fully so that all words are visible. If necessary, you can edit the title to be more concise while still conveying the full meaning.

@mercihabam
Copy link
Member

Also, please test every change you make. Does the current code still produce the intended behavior?

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

Successfully merging this pull request may close these issues.

2 participants