douggreen → created an issue.
I've been looking into the suggestion by @catch in #71. Here's a patch (no MR because I'm not sure we want to go this direction).
I'm sorry I didn't see these requests when they were needed a year ago. The best way to get my attention is through the contact for or slack :( I've committed the change for D11. TBH I don't use this module anymore and I'm not even sure who I wrote it for.
sure, I've granted you full access. tbh, I'm not even sure what client I wrote this for.
douggreen → changed the visibility of the branch 3497225-plain-text-formatter to active.
douggreen → changed the visibility of the branch 3497225-plain-text-formatter to hidden.
douggreen → created an issue.
This fixed the problem for us as well. Can we get this merged and a new tag applied?
Reroll for 10.4.x
Attached is #23 rerolled for core 10.4.x
douggreen → created an issue.
I've also run into another use-case where it doesn't work, still using IEF complex.
The last commit fixes the validation warning.
When I use this to just upload media, I now get the error This value should not be null.
after the upload succeeds, so there's something still wrong here.
I think this fixes it with IEF, but I haven't tested it outside IEF. Please test :)
douggreen → created an issue.
Oh wait, this will work, I just need to configure it differently. As for tests ... I apologize in advance, but I'll leave that for someone else.
@anybody this solution didn't actually solve our problem, so I'm going to suggest another solution. Our user's were still banned, but after a few tries now. I think we need config that just doesn't ban based on honeypot time.
Closing since this is included in ✨ Drupal 11 compatibility for Video Embed Field Needs review
I updated the project page with details from the README.
Closing, this was fixed some time ago.
merged, thanks
douggreen → made their first commit to this issue’s fork.
It does not. This is a very simple port of this one feature from the Drupal 7.x version of https://www.drupal.org/project/govdelivery → .
This appears to already be fixed (not sure when it was ever a problem TBH). The MR shows no changes.
Closing, D8 is no longer supported
This was done in June 2023, not sure why this issue still exists, closing
Committed, I wasn't initially a fan of this, but as an option it's certainly valuable.
sure, committed.
Thank you, this was merged.
This was committed directly. Thank you to the bot developers (having this issue and the bot tell me that nothing else needed to change, was empowering to make this simple change).
The MR adds configuration for honeypot. Do we also want to add a flag to just enable/disable honeypot banning?
I think it works with a Simple IEF form but not a Complex form.
There are also problems with IEF because the FormState built in the Event is missing critical state information.
We have also run into this. I think that the honeypot code needs to be more configurable. There should either be config to disable it, or config to control it's value separately from the default IP banning.
Closing, ... I ran into the same issue. We've decided to not pursue SRI for external JS provided by reliable 3rd parties such as yourself. While there is a security concern, it's impossible for us to keep up with 3rd party libraries which might change. This really is the responsibility of those 3rd party libraries, such as crazyegg, to provide SRI's.
I took a different approach than the existing work, and wrote a drush command that encrypts existing data. See ✨ Add drush command to encrypt existing data Needs work
douggreen → created an issue.
To answer my own question from above, see 📌 Create a Bulk Encryption/Decryption Service Class to facilitate encryption of pre-existing data Needs review and 📌 Create a UI for managing encryption of pre-existing data and changing encryption profiles. Active
The patch solves the error. I'm wondering if there's another solution to encrypt existing content that wasn't previously encrypted.
I've seen this warning in my logs but I'm having a hard time finding a test case, and I haven't seen it recently. I think that this patch is just hiding an underlying problem, which is that either that #max_delta is wrong, or that the element that #max_delta points to wasn't added. Can anyone reproduce this still? Is it possible that this was a problem in core that has since been fixed?
Patch attached
douggreen → created an issue.
LGTM
I still get a PHP warning in AttributeArray (line 79) because the original input array is statically cached in ViewExecutable::getExposedInput() on line 726, which happens before the data is validated. We need to validate the data before it is saved.
Since it's not version controlled, we can't rely on the SRI, and should remove it. The file has changed at least once since we implemented this.
I rebase and force pushed the branch. It needs review by someone who is familiar with what this does.
douggreen → made their first commit to this issue’s fork.
Do this do anything that ✨ Add option to show only start or end date in the DateTime Range custom formatter Needs review doesn't do? If it does, I think it needs to be re-worked now that the other issue has committed.
The above commit should fix this.
douggreen → created an issue.
douggreen → created an issue.
$ curl https://cdn.localizejs.com/localize.js | openssl dgst -sha384 -binary | openssl base64
7w/vCB9txvKaPgvRpSBQWnsULnvvH1VyQJXvjZujqTrsAvQI6B0H42gKqQUCG3y6
douggreen → created an issue.
I like the idea of removing the config option extlink_target_append_new_window_label which defines the label text and is new to this patch, always using the recommended text, and linking to the external resource (from above). This isn't removing an option in production. This is removing an option that someone in this thread thought helpful, and replacing it with a standardized (maybe industry standard) a11y text.
The current patch checks for "new window" in the existing label (not "opens in a new window") so I think that the existing patch is already backwards compatible with production systems.
If we agree, then what's left is to remove the config option extlink_target_append_new_window_label and make sure the tests work. If I understand correctly, we're pretty much at the place that you started with this ticket, it just took me a while to get there (sorry). What do you think?
This appears to have merged, so marking as fixed.
This was made an option because of comments starting in #11 above without any discussion until #51. Also, #51 suggested linking to WCAG somewhere, but that was never done. And IMO that is a better thing to have done than making the accessible feature optional. What should we link to?
While I dislike removing options that other people think are helpful, I agree with @jenlampton → that the new option extlink_target_append_new_window should not be an option but just always be true.
I'm a little less clear on whether the other new option extlink_target_append_new_window_label should be configurable or some set text. The fact that extlink_target_label is configurable makes me think that extlink_target_append_new_window_label should also be an option.
Also a little unclear to me is if the new JS I wrote should even exist. If both labels are configurable, and if the defaults are as we currently have them, this JS will never be used. And since the site builder has full control over this, is this new JS a waste of time to run in the browser and maintain in code?
There is some overlap (and conflicts) with ✨ A11y: add screen reader text for external links Needs review so I copied this commit from here to there. This can still be merged, if it goes first. Or if that MR goes first, this can be closed.
There is some overlap (and conflicts) with 🐛 Aria-label for external links span throws errors on Accessibility Arc Toolkit Needs work so I copied that commit from https://git.drupalcode.org/project/extlink/-/merge_requests/18 to https://git.drupalcode.org/project/extlink/-/merge_requests/16 and resolved the conflicts.
I created MR https://git.drupalcode.org/project/extlink/-/merge_requests/18 so I'm removing the patch attachments from this, to avoid confusion.
douggreen → made their first commit to this issue’s fork.
I created a MR, so I'm hiding all of the patches attached to this ticket, to avoid confusion.
The attached patch :
- makes the "(new window)" label configurable
- compares the title and the "(new window)" label in a case-insensitive manner without the parenthesis. See Drupal.extlink.compareLabels().
- combines the two titles in a more readable manner, see Drupal.extlink.combineLabels().
It does not update the tests.
douggreen → created an issue.
In response to #3 - You could keep logBlockedSubmission in the native condition, that's a design choice. I like using my own hooks as it shows other developers how to use them.
In response to #4 - we aren't altering anything, I think invokeAll is the correct method. And yes, we should inject the module_handler.
Committed, thanks!
douggreen → created an issue.
douggreen → created an issue.
I've seen a similar error in my logs, but I'm on Drupal 10.2.x (not 7.x as this issue is against), so this is not the right fix. And I don't think the problem is passing an empty path to the delete, because my log shows the 1.xml in the path not being deleted.
This is easy to test. Create a URL that sends the lat and not lng or vice-versa. I normally agree that we shouldn't hide PHP warnings, except when they can be triggered by a user sending bad data. It's a simple fix, and I disagree that this is how it is intended to work. We should fail, but not fail with PHP warnings.
merged, thanks!
Thanks, I've updated the project page and marked the project as Unsupported.
sure, thanks for the help!
Odd, I don't see the "create Merge Request" button. I created a branch and pushed to it but the "create merge request" button is missing and the link that git gives me gives a 404.
https://git.drupalcode.org/issue/drupal-3412420/-/compare/11.x...3412420...
MR created.
In reply to the comment by @smustgrave that this was possibly done on purpose because of a layout builder scenario, that isn't mentioned at all in the original ticket. And this very problem here was mentioned there.
This didn't fix create access. See 🐛 BlockContentAccessControlHandler requires access block library permission for create Needs review
Patch attached