🇺🇸United States @pianomansam

Because this module includes a submodule for Webform, we will need to wait until Webform has a Drupal 11 release.

Updating to postponed as more information is needed from the OP to resolve this.

@heliogabal I'm sorry to hear the module isn't working for you. The scenario you describe sounds pretty complicated and won't be easy to replicate. Can you provide a simple scenario where you aren't seeing it work? Perhaps the module does work in simpler scenarios but not complex ones. We'll need to narrow down exactly where it breaks down.

Drupal 10 has, yes. But this module supports Drupal 9, which is had ES5 as its standard. If a module is to support Drupal 9, I think it should also support all the browsers that Drupal 9 supported.

https://www.drupal.org/node/3305487 →

I was experiencing this issue when using the 2.x branch. When putting a console log on the JS click handler, the console output my message 16 times for a single click. When I looked closer at the code, I realized the JS reapplies the click handler each time Drupal does a JS attach. Which in my case, was 16 times. I've created an MR that uses both the core/once library as well as Drupal attach's context to make the click handler only occur once.

pianomansam → made their first commit to this issue’s fork.

Changing the status back to needs work as there are unresolved questions on the merge request. Further, the merge request doesn't do anything to solve the issue raised in #23. The fix in #23 is still a manual fix that should be taken care of programmatically with a migration/update.

@jayhuskins thank you for the bug report and initial MR. For it to be accepted, please create a failing test demonstrating the issue and a passing test.

@i-trokhanenko that's awesome. Thanks so much! With this out of the way, it would fantastic to see progress on ✨ Make compatible with simple_sitemap 4.0 & domain_entity Needs review

Can one of the 6 maintainers look at this? Drupal 9 is now unsupported. What needs to happen to get this across the finish line?

pianomansam → changed the visibility of the branch 3379202-drupal-10-compatibility to active.

pianomansam → changed the visibility of the branch 3379202-drupal-10-compatibility to hidden.

This was included in ✨ Support for webform Needs work

pianomansam → made their first commit to this issue’s fork.

I got tests to work, so I expanded them to make sure they handled both current-user and current-page tokens. Sure enough, they caught an issue with current-user tokens. That is now resolved and I'm content with the amount of test coverage, so I'm merging this into dev.

@JeroenT any chance you can help get Gitlab CI working on ✨ Support for webform Needs work ?

@jmaties While I'm trying to get the testing rig working with the submodule, it looks like TokenOrWebformFunctionalTestBase is throwing a deprecation error in Drupal 9:

Calling Drupal\Tests\WebAssert::elementTextContains with more than three arguments is deprecated in drupal:9.1.0 and will throw an \InvalidArgumentException in drupal:10.0.0. See https://www.drupal.org/node/3162537 →
1x in TokenOrWebformFunctionalTestBase::testGetFirstParam from Drupal\Tests\token_or_webform\Functional
1x in TokenOrWebformFunctionalTestBase::testNotGetParam from Drupal\Tests\token_or_webform\Functional

While I see tests for the submodule, it doesn't look like they are being run by the continuous integration.

@jmaties thanks for your work on this. It looks like this is still in "Needs work" but I wanted to give some quick feedback.

First, it looks like you have duplicated the entire replace method. It would be great if this was more surgical, only making the changes necessary for this functionality to work, and then handing it off to the parent. The reason for this is that the underlying replace method in WebformTokenManager could change at any time. If/when it does, those changes won't be reflected in our version. Thus increasing maintenance and the chance something could break.

Second, to be accepted, we'll need tests added to your MR. Initially, we'll need tests that fail and demonstrate the issue. Then we'll need passing tests that demonstrate it is resolved.

This issue appears to be fixed in 8.x-1.3

Yes, that appears so. Closing.

Specifically, it's not clear to me how to do this step:

You will also need to call this module's AnonymousCsrfTokenGenerator service that wraps the CsrfTokenGenerator from core in order to complete the implementation

I too am struggling with getting this module working. Perhaps a small example module could be added to the project?

If #2274177: Allow altering settings per editor and field → will fix this, better to make this a duplicate and track the other issue, because at this point in time, the issue is not fixed.

@darrenwh please read Smart IP page under the "Configuration / Cache Performance Note".

Am I blind, or does that section no longer exist? I, too, would like to know how to get this running behind Varnish or nginx

I was wondering if it was the token filter module. What's the order of filters? If the token filter first?

@facine Hmm, can you provide a bit more context? Is a text format involved? Any particular modules or contexts where this is happening?

@facine, thanks for the updated patch. I'll review it.

Also, thanks for the scenario where this could be an issue. Is this particular one breaking something for you? Whether you use <a href="mailto:[current-user:mail|'email@example.com']"> or <a href="mailto:[current-user:mail|"email@example.com"]">, the result will be the same:

<a href="mailto:email@example.com">

@facine can you also provide an example of how this affects you? The token replacement will also replace the double quotes, so it's unclear to me how this is an actual issue.

@facine I think adding this feature is great. As it is currently written, though, it looks like there's nothing that forces the content to use the same type of quote character; a double quote could be first and a single quote second.

pianomansam → created an issue.

I've created an issue fork and pull request that does what I propose to do. It only uses the optimizer if CSS preprocessing is enabled.

pianomansam → created an issue.

I was able to reproduce this issue. Apparently, some modules directly send NULL into the token service.

pianomansam → made their first commit to this issue’s fork.

@fromme testing with the quiz module sounds pretty involved, so I'm not sure it will work here. Out of curiosity, what's the token you're using that gives the error?

@fromme thank you for your report and patch. I'm having a hard time reproducing this. Before a patch can be accepted, we need a test that can reproduce and test that the patch fixes the issue. Can you at least help me reproduce it?

Patch is also working for me. A release should be created.

The code is now available in git. Closing.

Updated with null coalesce

Created a patch to check that $credentials is an array before returning it.

pianomansam → created an issue.

Thanks!

I've created an issue fork and merge request to resolve this. I've tested this on both page and block view displays.

Attaching possibly related issue ✨ Should refresh work on the view block displays? Active

pianomansam → created an issue.

pianomansam → created an issue.

I have reviewed the code for both projects, and they are nearly identical. So I'm changing this issue to be a request to merge the two modules into one for the sake of the community.

pianomansam → created an issue.

Can we get a release with this fix included?

Here is a patch that adds these two fields as configurable values.

pianomansam → created an issue.

Here is an attempt at a patch.

pianomansam → created an issue.

Reworked patch to apply to 10.x

Can I ask why the introduction of the menu_link module dependency?

pianomansam → created an issue.

@captain hindsight I think your solution could work. Feel free to create an issue fork to test that out.

Under the current described workflow it would seem once an approver sets the scheduler if an editor can go in and edit then they can get around the reviewer and publish what they want.

That would only happen if the editor has permission to perform a workflow transition that would publish it. In my scenario, the permissions on workflow transitions don't allow a non-admin (editor) to perform any transition into a published state.

Here's the list of my workflow states:

1. Draft
2. Needs Review
3. Scheduled
4. Published
5. Archived

Non-admins (editors) have permission to perform the following transitions:

1. Save Draft (Draft, Needs Review -> Draft)
2. Submit for review (Draft, Needs Review, Scheduled, Published, Archived -> Needs Review)

Admins (approvers) can do the above but also:

1. Schedule (Needs Review -> Scheduled)
2. Publish (Draft, Needs Review, Archive -> Published)
3. Archive (Published -> Archive)

However, because scheduler_content_moderation_integration_entity_access() limits the transitions to the ones stored at $entity->publish_state and $entity->unpublish_state, non-admins (editors) cannot perform either of their transitions.

So again I ask, why does this module tinker with workflow transition permissions? If the transition permissions are incorrectly configured, that's not this module's fault. IMHO, this module should only worry about scheduling one transition to another, not changing how the core transition permissions work. Unless it has a certain reason to do so, and then only if the opted into.

This fixed the issue for me on 2.0.0-beta1. I'm bumping this issue to 2.x-dev because that's the latest version. However, it can be backported and the current time as well.

Let's get some tests around reproducing and solving this.

-  public function tokensPreAlter(&$text, $data, $options) {
+  public function tokensPreAlter(string &$text, $data, $options) {

@binoli-lalani I'm not sure this is the best approach. How can we be sure that the code upstream only passes in a string? In fact, this error report indicates that it's passing in NULL.

I just ran into this PHP 7 incompatibility. What can be done to address this?

Confirmed. Marking this as a duplicate of 🐛 Drupal\color_field\Plugin\Field\FieldFormatter\ColorFieldFormatterSwatch::getShape(): Return value must be of type array|string, Drupal\Core\StringTranslation\TranslatableMarkup Needs work

I encountered this, too. I'm on PHP 7.4. This is the line in question:

protected function getColorFormat(?string $format = NULL): array|MarkupInterface {

It looks like this is an issue with PHP 7 backward compatibility.

pianomansam → created an issue.