This patch fixes the issue. The functionality appeared here - https://git.drupalcode.org/project/media_directories/-/commit/62f2edcc1e...
Thank you for reminder, release 2.0.1 created.
Sorry, I've put early version of the patch in #16, please use this for 3.x
The patch isn't applied to 2.0.9, just adapted it.
I propose to use "pageshow" event, it is fired just after a page load and has "persisted" property to check if the page is rendering from cache. Also I propose to re-render checkboxes for a full sync of its state to the related link, which avoids both issues with disabled and checked states. Please try the patch.
Checking proposed solutions I've found:
1. None of them work in Safari.
2. The window.unload event is deprecated (https://developer.mozilla.org/en-US/docs/Web/API/Window/unload_event), and window.onbeforeunload fires too early. When adding 'sleep(3)' to the index.php, I observed that the checkboxes become enabled again, allowing me to click them once more.
3. If checkboxes aren't disabled at all, it leads to #2658678 behaviour.
Looks like #2658678 issue was solved with checkbox disabling functionality.
The problem was already tried to be solved in #3210353 issue, relating it.
Patch #3079534 works, but there is also ability to do the same with internal module changes. Please test the patch.
Duplicates #2929733 🐛 SQL query error on PostgreSQL 9.5 when try to list Bookmarks Active older one.
Thanks, I reviewed issues and I see it is a duplicate of #3260731 → and #2929733 🐛 SQL query error on PostgreSQL 9.5 when try to list Bookmarks Active . They have no patches, I'll add the patch to the latest one and close another one.
Other issues as I see are related to DB field type change to int, I remember your answer on this point.
Core views issue as I see also should resolve it.
By the way, why Drupal 'http_client' (or 'http_client_factory') isn't used to support system-wide config, including proxy?
I propose the next solution, please help with tests if needed.
I'm not sure that collect "terms" key is universal solution. It definitely works with simple string facet, but it could have issues if "exclude" options is enabled in facet or it is range or match facet.
I propose to collect all the filters and apply it to "agg" syntax. If filters works with "query" section so it should work here too.
Need to be tested with complex, negation facets and with ones with "exclude" option configured.
Looks like "Converts CSS styles into inline style attributes." feature isn't implemented in D8+ too. Please add notes to the module page, because the feature is important for us and it was the main point to try this module. For a now I'll try to attach "twig/cssinliner-extra".
The patch
Here is a patch that solves the issue, it works with MySQL too, but I didn't checked if it has any affect on performance.
Hmm, good point, but I think separate permissions aren't needed. The popup view could be shown public or with "access published content" permissions.
@ujjwal_singh could you please provide steps to reproduce? Because "domains" goes from drupalSettings directly and it is always string, it splitted in JS before be passed here. I can't reproduce it.
@berdir, I've just like to write that this patch solved issue with view query (and possible issue with content deletion), but another issue happened - now there is error on PostgreSQL on config entities deletion, because it has string ID, and `entity_id` column is numeric now. Looks like there are 2 ways:
- Support content entities only with numeric IDs (huge change, but by the way, why flag initially supports config entities? Its look as really rare case to flag e.g. node type or view).
- Try to normalize all entity ID numbers to strings before pass to queries (may be I'll try to do it on Monday, first I would like to check Voting API, which uses "entity_reference" field type with dynamic numeric/string column type detection)
Non integer IDs cause issues using PostgreSQL:
SQLSTATE[42883]: Undefined function: 7 ERROR: operator does not exist: bigint = character varying LINE 4: INNER JOIN "flagging" "flagging_taxonomy_term_field_data" ON taxonomy_term_field_data.tid = flagging_tax>
P.S. Looks like the issue has same patches to https://www.drupal.org/project/flag/issues/2888664 🐛 New index for flagging performance in views relationships Needs work , install hook there contains a bit more operations.
Without the patch there is an error using PostgreSQL DB:
SQLSTATE[42883]: Undefined function: 7 ERROR: operator does not exist: bigint = character varying LINE 4: INNER JOIN "flagging" "flagging_taxonomy_term_field_data" ON taxonomy_term_field_data.tid = flagging_tax>
Module: 8.x-4.0-beta5
Drupal: 10.3.2
PostgreSQL: 16.3
Patch from #41 solved the error.
Still having the issue, docker image docker.elastic.co/elasticsearch/elasticsearch:8.14.3
Version: 8.14.3, Build: docker/d55f984299e0e88dee72ebd8255f7ff130859ad0/2024-07-07T22:04:49.882652950Z, JVM: 22.0.1
Drupal 10.3.2, module version 8.0.x-dev
2 facets, string (list string field) and entity reference, some combination are working, but most of combinations leads too:
{"error":{"root_cause":[{"type":"parsing_exception","reason":"Expected [START_OBJECT] under [filter], but got a [START_ARRAY] in [field_content_reference_filtered]","line":1,"col":358}],"type":"parsing_exception","reason":"Expected [START_OBJECT] under [filter], but got a [START_ARRAY] in [field_content_reference_filtered]","line":1,"col":358},"status":400}
Please try this one
It isn't the best solution, by the way a problem is in one mistake defining theme array for suggestion, I'll provide a patch in the next message.
Here is the first version of patch that works with fresh installed module only (or you need re-config taxonomy term relations manually). Hook to rename existing configs need to be implemented in "HOOK_post_update_HOOK". I'm going to make the improvement a bit later.
Thank you, merged to dev version. I make release a bit later.
One more case:
Having Drupal installed in subfolder, going from content overview page to translation page (URL looks like http://example.com/web/node/2/translations?destination=/web/admin/content), after submitting translation I go to 404 page (http://example.com/web/web/admin/content).
The next patch from the child solves this issue too.
Patch from related #3436477 solves this issue too.
With this changes to use RedirectResponseSubscriber for redirects when destination is set it solves parent issue too.
Also I assume the module has issue with redirects when destination is set on sub-folder installation, looks like test doesn't cover the case. I'll check later and rename a task if needed.
If base url exists, its added one more time for Url set to form redirect. The easiest way to fix its to return destination to query bag, setting empty 'base_url' in Url object params looks tricky for me.
With a patch in #3 there is an error if no destination query param is provided on Translations page
InvalidArgumentException: The internal path component 'node/179158/translations' is invalid. Its path component must have a leading slash, e.g. internal:/foo. in Drupal\Core\Url::fromInternalUri() (line 416 of core/lib/Drupal/Core/Url.php).
Patch below process path after UrlHelper::parse(), which supports both URIs with and without leading slash.
During a patch creation I found that the module use "base:" protocol to get URL, the error is caused by #3319509 patch. Anyway I think its a good improvement to use RedirectDestinationTrait, so I keep patch below. Also there is a chance that #3319509 would be fixed in this way.
dewalt → created an issue.
Thanks @himanshu_jhaloya, but "::condition()" methods doesn't support MySQL expressions, simple query is provided as example only. For a cases when expressions are used in conditions looks like only "::where()" method works, e.g. "WHERE BINARY value = IN (:arg[])" or for binary (case-sensitive) columns "WHERE LOWER(value) IN (:args[])", and many other possible expressions.
dewalt → created an issue.
Yes, the issue isn't reproduced more, affected code '#prefix' => '<div class="ajax-progress-throbber"></div>', was removed. In my patch unused library "entity_browser/iframe_selection" was removed that was introduced in related issues.
I decrease a priority to minor, and the issue is "should unused library be removed"?
Updated #56 patch to use with latest module version.
The patch is merged, should be the issue closed?
I confirm that cache tag invalidated twice, but the issue with caching could take a place. The cache tags are cleared as a last action in the method (generally as should be), but exception thrown during indexing or results post-processing in $dispatcher->dispatch(new ItemsIndexedEvent($this, $processed_ids), SearchApiEvents::ITEMS_INDEXED); leads that execution stopped and cache tags aren't invalidated.
In addition if "index_directly" option is enabled, indexing takes place after response was sent. In this way user doesn't see that any error was happened. It could be found in Drupal log, but if it is PHP error (out of memory/time limit), it could be found in server logs only.
In this way from user viewpoint everything should be done, but cache isn't cleared and outdated results are shown.
So, this is a question, should the cachetags be invalidated twice before and after indexing, or should try/catch be added (doesn't help on PHP errors), or it is and environment case and generally works as designed.
Looks like the maintainer solved this issue in this commit 2 mount ago and left you without credits(
Could you please provide a link to the release notes it was removed? I'd like to read before creating a feature request.
@saschaeggi I see, but in this way it should be removed from "menu--toolbar--gin.html.twig" where it still exists.
@larowlan, I've made research of the issue and tried to provide explanation in #131. If anyone have some exact questions - you are welcome.
@neclimdul Do you propose just to add the Unit test from #130?
@rang501 - the view already was fully overridden recently in media_directories_ui_update_9002(). It isn't good and was a surprise for us, but if it is already done I think it could be repeated.
Guys, it could be funny to read "Unsure about pagination ... Locally, with 1000 media entities" in pager discussions if it isn't a problem. A view fully loads each entity, and having no pager its only a question of entities count and allowed memory when you'll get a "500: Allowed memory size ..." error.
After last module update the media_directories_base view was overridden in an update. Previously we customized the view with pager and sorts. And after the update we've got "Allowed memory size exhausted" error. We have 512M memory limit (higher than minimal recommended) and ~10 000 media entities only. It isn't much, I worked on projects where were hundreds of thousands media entities.
The pagination is must be implemented. And looks like it is supported from the box - I've just added "full" pager into the view an everything works.
For the 1 case "advanced_help-replace_full_html_unsafe-3404344-2.patch" is ready.
For case 3 "advanced_help-replace_full_html_with_xss_filter_admin-3404344-2.patch" changes output, but don't make changes to example module.
Just adding diff from the MR to use with composer. The diff applies for 1.15 modules version, but has conflicts with the latest dev version.
Oops, missed reject on another test. Fixed.
The last patch isn't applied on a newer module versions, updating it.
@Berdir, what is you feedback about the changes? The only affecting change is to forbid anonymous access by default on the module install, changes in hook_entity_access() is an improvement (previously the context was added even if the config option is disabled). But it affects a fresh module installations only, all existing installations wouldn't change its config on the module update.
If the setting is needed for some specific provider I can create ticket to enable the setting on a module install. Also if anonymous access is needed for some specific pages only (e.g. /node/ID) - its good idea to allow access on this pages only.
I'm not sure what encoding is needed for filename inside of "Content-disposition" header, in Chrome and Safari it works with backslashes, URL encode and plane, even if filename has quotes. So I keep it plain, same to implementation in the module in DataExport::buildStandard()
The issue takes place when export files are stored to the private location. Hook "hook_file_download()" allows add headers to private files. Patch below adds the hook to the module. Also with this change JSON download also works in Safari.
Here is a patch, I've removed "post-check=0, pre-check=0" from "Cache-Control" header as looks it isn't actual more.
driverok → credited dewalt → .
I propose also to add new index on node_field_revision table, in my case it extremely increase a performance:
- revision_translation_affected
- nid
- vid
Looks like temporary table constructions in $subquery isn't cached by MariaDB engine. We have match content and more that 100k revisions. Having a dashboard with 4 node revision views (each use "Is Latest Translation Affected Revision" filter) I have the next times of the page render (I make `drush cr` before each load):
Core: 7m 15s
Core+index: 40s
Patch #25: 3m 40s
Patch #25+index: 5s!!!
Thanks for patch, generally it works, but I'd like to propose some improvements.
1. The last "continue" is in one more foreach, so I think there should be
continue 2;
2. "array_pop" removes the last element in the array, and it isn't validated in further foreach cycle.
$token_array = [array_pop($parts) => $replacement];
It could be fixed in many ways, like
// Its isn't good from design perspective as variable changes type from sting to array, but works in PHP.
$token_array = $replacement;
or
$token_array = NULL;
// ...
$token_array = [$key => $token_array ?? $replacement];
I don't support an idea to beatify all the file (not the changed lines only), I think doc-comments should be added in a scope of other issue, but not in a bug. By the example from previous patches I've just add `$value = NULL;` to pass PHPCS, rebased to 11.x branch and no more changed from #137.
P.S. by the way PHPStorm not highlights the variable as unused(
10.x patch should work for 11 branch too.
The reason is described as "If the site is offline, log out unprivileged users." but I still can't understand why we should force the logout for all users, instead of just showing the "Site is offline" message on the blank page?
Imagine, you use public computer, and the site occasionally goes in maintenance. You need go away soon, but you have no ability to log out. As a rule typical user have no skills to clean up cookies, use guest mode in public computers, etc. In this way the next user would see your account, and could get you PI data or compromise you on this site, delete account, etc.
The issue could be solved providing access to "Log Out" action in maintenance too, but looks like that Drupal just uses force-logout.
Looks like for the bootstrap theme alter is needed in custom code to unset dependency. The patch breaks functionality without the theme.