Please add the return type declaration.
Yeah, I wasn't sure if this issue should be closed or postponed, or what.
Leaving open in case anything else comes through via the bots.
Merge conflicts exist in the MR due to another issue getting merged. Please update the MR.
I feel like this is an unnecessary bloat to the already complex form. And comments can often be placed directly in the JS/CSS asset. If there is a desire for notes field, it would be straight forward to modify the entity type to add the field.
The MR seems to address more testing than the actual feature requested, and the feature requested is entirely just a cosmetic change the form. No functionality changed. This works for me.
MRs aren't merging for me, so I'll commit and push up
Thanks for this.
I seem to be getting "An error occurred while merging. Try again." on all MR. I've commit the old fashion way.
I like this new approach. I hope it solves the s3 issue as well.
A couple things to change in the MR:
add docblock & return type to the new getDirectoryUri method
There's no need for the "else" condition. Something like below will be more streamlined.
if($is_true) {
return 'foo';
}
return 'bar';
Hi @anybody,
I appreciate the assistance in the queues. I've been keeping an eye on anything that looks urgent. Besides the assets:// wrapper, I haven't seen much that has provoked me to jump in. There are some great discussions and work in those issue queues, but for now, I would prefer to keep a close eye on it and avoid additional maintenance burdens. I have some thoughts about refactoring the module into a bundle entity type and possibly allowing content storage along side config storage.
Please continue to contribute through the issue queues and I'm happy to revisit the request at a later date.
Thanks.
Since it seems the MR patch url isn't working correctly (it seems outdated), here's a patch file that applies to both 1.x and 2.x branches. It's the same as the MR.
Patch in #5 solved it for me. I wouldn't think there needs to be a canonical path for encryption profiles anyways.
I've been just ignoring the error for a while and finally decided to see if there was anyone with the same issue. Glad I waited until today.
Any security concerns are handled by the security advisory process → . Also, as mentioned on the module information "This module is definitely not a replacement for full-fledged theming" and standard theme development is recommended for long term stability.
That being said, any concerns with storing the CSS/JS in the database is arguably minimal compared to any other data stored, such as user data, privileged content data, etc. If a user has access to manipulate data in the database, they have greater access to obtain more sensitive information. The permissions to edit via the UI is configured to be "restricted" and therefore there is a warning when granting permissions that there is a security concern with the message "Warning: Give to trusted roles only; this permission has security implications", therefore, any risk is assumed by the site administrator granting such a permission.
Can you encrypt the data that is stored in the DB, sure. You can implement your own hooks such as hook_entity_presave and hook_entity_load and encrypt/decrypt the data stored in the DB. If you are concerned about the risk of the generated CSS/JS files, I will refer back to my first point and suggest standard theme practices. Drupal's CSS/JS aggregation as well as browser loading requires a static file to be available, so there's no other option than to have these saved to the file system upon page load.
pookmish → created an issue.
Thanks for the reference to the Core issue, @el7cosmos. That makes more sense why this issue is happening. It looks like your MR works well for me. I was able to do a full site install with a memory limit of 512MB. To be thorough, I tested with 256MB memory limit. Both your MR and the patch in #7 ran OOM. I like your approach better as well since it maintains proper dependency injection in the service.
Thanks.
Since the above patches required re-rolling to support the latest version of the module, I double check the memory issue. It still occurs and happens with both services. I tried to leave each of the services, but the OOM would still happen.
@el7cosmos this is tough to test, but it seems like others are having similar issues. If you have an installation profile that can install from existing configuration, it should be replicated. Lower your CLI memory limit to something. For my situation, even 512M caused the OOM error.
Using the MR33 I got a warning when the image is first uploaded:
Undefined variable $webp_wanted in Drupal\webp\Controller\ImageStyleDownloadController->deliver() (line 192 of modules/contrib/webp/src/Controller/ImageStyleDownloadController.php
I think it would be better to break out the "if" statement just a little. Since I'm faster with patch files at the moment, I've provided one with an interdiff.
@hswong3i, This sounds like you have something else going on. In the 8.x-2.x version, line 153 is a comment and the only thing added in the commit was adding a hook_preprocess_maintenance_page which works exactly like it's supposed to when using a base installation. You should debug your code first.
Providing a patch for the latest version of migrate_tools.
This issue breaks all Drush commands with these two module. This is a critical issue.
This is really a duplicate of 🐛 "Drupal\migrate_tools\Commands\MigrateToolsCommands" not found PathRedirectImportCommands Postponed: needs info
Providing the patch for 8.x-2.x version
pookmish → created an issue.
Looks like the test changes from #16 were actually implemented in another change.
Rerolling the patch with just the .module change for D10.2.0+
Sufficient, probably. It think of the dependency as a belt and suspenders approach.
pookmish → created an issue.
The provided patch does not fix anything. When using the provided field widget only (not the submodules at all), the classes for the icons in the autocomplete do not match FontAwesome 5 classes. They do however match FontAwesome 6.
Take for example the `drupal` icon: Using FA5 the appropriate classes are 'fab fa-drupal'. But in the autocomplete controller, the brands are prefixed with 'fa-brands'. This matches the FA6 version of the "drupal" icon though.
The good thing for me is that the displayed classes using the included field widget render correctly.
I do believe this module should definitely support the FA6 classes, but also the FA5 classes.
pookmish → changed the visibility of the branch 3406342-support-php-8.2 to hidden.
Patch file provided for anyone that may need it.
pookmish → created an issue.
pookmish → created an issue.
I believe this is outdated now with the 8.x-3.0-rc1 release. I am able to use wildcard force imports without the need for any patch. If anyone can confirm, this issue might be closed.
The patch in #3 didn't apply for me. I think it's overly complicated anyways. Attached is a solution I was able to use.
Sorry, wrong error/module
pookmish → created an issue.
This seems to work for me. But I didn't review very deep about the table structure.
pookmish → created an issue.
pookmish → made their first commit to this issue’s fork.
I correct myself in #8 (unless it was some dependency update I needed). I've attached the patch that works for me with Drupal 10.1.4.
@noemi, I wasn't able to reproduce your incorrect count. If I set `?page[limit]=5` the returned number in the meta is 5. What could be more helpful is to have a "total" returned as well. But I wasn't able to find a quick solution for that.
The suggested patch in #5 does not work in Drupal 10 due to symfony's ->get() method on the input bag. The input bag now requires the returned data to be a scalar (string, number, boolean, etc). But switching to use the ->all() method further breaks due to the Drupal core's view loads the pager via this line in PagerParameters. The patch mentioned in #6 and 7 does not help because it uses the same ->get() method and throws the same error, just in a different place.
Would something like Custom Meta → satisfy that request? Seems to fit the request to me.
Hi @bessonweb, please try the latest 2.19 release. A change was made to the update hook that should solve this issue.
Ahh thank you @codebymikey
Is "seven" theme enabled? If not, you will need to adjust your config manually prior to the update.
You will need to provide more information.
pookmish → made their first commit to this issue’s fork.
The current_theme issue was noted in https://www.drupal.org/project/asset_injector/issues/3329577#comment-148... 🐛 Fatal error: Schema error for theme condition selection field Fixed
Note that the current_theme was still working with multiple values, but it was causing errors when saving. That was "fixed" with this commit. At the time of fixing that, there wasn't a need to provide an update hook since the conditions still worked. If that is no longer the case, please open a new issue.
Re-rolled the patch from #22 due to the 1.9 release
pookmish → created an issue.
Something like this should have generated a new minor or major release. a patch release is insufficient for such a refactor. This require developer actions beyond just a normal composer update. You should consider the sites that have the module enabled would be broken due to removing the various sub modules.
I needed this with Drupal 10.1 as well. MR worked for me.
Looks like this issue was actually committed in the dev branch. Can we please get a release to provide the upgrade path for users of this module?
Looks like this starts at the container resolver and then issues propagate out to all the hooks. This likely will require some extensive refactoring.
pookmish → created an issue.
#18 works for menu link content. But when using menu_link → the problem still exists. We are using that module in conjunction with menu_link_weight → . I think it might be helpful to have some type of configurable settings that allow this to work. Then users can customize which providers they wish to allow customizing.
I agree with KarlShea. There needs to be much greater documentation about the what happens when installing both modules at the same time. The current language provides no helpful information. It required actually tracing hook implementations for me to find where the conflict was.
I'm not sure if OP's issue is related to mine. I found that if I enable `use_collection` for multiple containers, the module will still only add the first container. So I have to put all my GTM IDs into the one container for them to load as expected. This kinda defeats multiple containers.
It might also be that OP isn't running the update since they said reverting solved the problem.
pookmish → created an issue.
Ultrabob, nobody said anything about in this issue related to uninstalling the module. This is specifically when deleting an item. Maybe it could be related but that's never been mentioned until your comment.
pookmish → created an issue.
I believe this should actually be an issue with google_tag module. It contains a hook_module_implements_alter that unsets the google analytics hook which adds the GA code.
pookmish → created an issue.
pookmish → made their first commit to this issue’s fork.
pookmish → created an issue.
This issue is still a problem on the 2.x version. The issue was resolved in 1.x but not 2.x.
Hmm. that is an issue. I wonder if this is an issue with drupal's package manager... I wonder if there are other examples of this scenario.
You would think so, but because the Fast 404 module's info.yml file is `fast404.info.yml` the dependency is correct.
https://git.drupalcode.org/project/fast_404/-/blob/8.x-2.x/fast404.info.yml
pookmish → created an issue.
Raising priority because this is required to get Drupal 10 functional and fully featured as an upgrade from the 1.x versions.
pookmish → created an issue.
Reopening because when updating the menu_link_weight module, it requires this module. Then it breaks the access to the menu fields on the node form. Since this module provides no permissions, it is breaking the intended functionality of "Menu Admin Per Menu" module.
I don't want to provide the "Administer menus and menu links" permissions because that grants access to menus they shouldn't have access to.
The patch in #7 works for me. How is this issue 3 years old and still an active issue?
Patches from the update bot don't apply anymore. I've attached a new patch and tested some simple validation rules on Drupal 10 with success.
Please prioritize this for a new release for Drupal 10 compatibility.
I'm providing a patch that appears to work for me on Drupal 10.
Note that the react todo example didn't work, but the block is rendering and the libraries are being added to the page. The React "Hello World" did work as expected.
Please prioritize an update to the module for Drupal 10 readiness.
Confirmed the module still works as expected on Drupal 10. This patch is very minimal. Please apply the patch and release a new version of the module for Drupal 10 readiness.