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 is not enabled #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
@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?

@marclaporte
Copy link
Member

@christer77 was unavailable but he is now back.

@christer77 christer77 force-pushed the get_list_block_sieve_to_sieve_module branch from 2551331 to 68ba47e Compare March 13, 2025 09:23
@christer77 christer77 force-pushed the get_list_block_sieve_to_sieve_module branch from 68ba47e to 9928933 Compare March 13, 2025 09:31
@christer77 christer77 changed the title fix(frontend): get_list_block_sieve function not callable if sievefilters module… fix(frontend): get_list_block_sieve function not callable if sievefilters module is not enabled Mar 13, 2025
@christer77 christer77 requested a review from mercihabam March 13, 2025 09:35
$factory = get_sieve_client_factory($this->config);
try {
$client = $factory->init($this->user_config, $imap_account);
if ($this->module_is_supported('sievefilters') && $this->user_config->get('enable_sieve_filter_setting', DEFAULT_ENABLE_SIEVE_FILTER)) {
Copy link
Member

Choose a reason for hiding this comment

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

These checks do not belong here. If that code is executed, the sievefilters module will already be supported and enabled. My emphasis was on the browser code, particularly the get_list_block_sieve() function, which is called in the messageList page handler; see line 85 in modules/core/js_modules/route_handlers.js. This function executed successfully because you had it previously defined in the imap/site.js, it kept being executed but not with successful result when the sievefilters module was disabled. You moved it to the right location, but now, if the sievefilters module is disabled (preventing the browser from reading any code from modules/sievefilters/**/*), the messageList page handler will throw an error because get_list_block_sieve() is not defined.

We do not have an actual way to detect which module is disabled via the JavaScript code, so what we do is to ensure the function is globally defined before it gets executed. Here is an example with the savedSearch page handler.

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.

3 participants