Account created on 5 November 2008, over 16 years ago
#

Merge Requests

More

Recent comments

🇫🇮Finland sokru

The Pathauto module works already like this, if the pattern (defined at /admin/config/search/path/patterns) is [node:title], the the page with title Test《》page, creates an alias testpage. If you want to replace the character (transliterated into < and >) with separator, you can define that on admin/config/search/path/settings under "Punctuation" details element.

🇫🇮Finland sokru

What's the correct transliteration for the given symbol? Drupal core's transliteration service gives transliteration <<>>/, and those characters are not valid for URL, therefore they are removed.

Changed the priority to correspond the Drupal definition: https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-issue/issue-priority-field

🇫🇮Finland sokru

Yes this looks like a duplicate. I'll try to review the MR on the other issue.

🇫🇮Finland sokru

Fixed small lint issue on composer.json. Setting status RTBC and leaving up to maintainers to choose if they want to support D9/Drush11.

🇫🇮Finland sokru

sokru changed the visibility of the branch 3471063-modernize-drush-commands to hidden.

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

Please re-open the issue if you are able to produce steps how to reproduce this.

🇫🇮Finland sokru

I tried to replicate the issue with Drupal 11 and Pathauto 1.13, but didn't manage with steps from the issue summary. I also tried to switch steps 7 and 8, but no difference.
The steps didn't mention the language to use on steps 8, 9, 13 and 14, but I assumed it to be the default language (English).
I'd recommend checking the path alias (steps 13/14) from route /admin/config/search/path

🇫🇮Finland sokru

- I hid the 11.1.x branch
- I moved show_link_as element into trait. If we can think up something better name for "UrlSuggestionTrait" be could change it now.
- relativeUrlSuggestion() had absolute URL in #description, changed it to relative
- Based on #138.2 and #138.3 I removed the unnecessary description text "If checked, links will be rendered as absolute URLs."
- I bumped the versions on deprecated message

Changing the status to "Needs review" although there's open questions from @larowlan

🇫🇮Finland sokru

sokru changed the visibility of the branch 2811043-file-formatter-render-11.1 to hidden.

🇫🇮Finland sokru

@reszli thanks for the workaround, I tried to revert #2826021: FieldItemList::equals is sufficient from the storage perspective but not for code checking for changes but didn't have an effect.

Updated the IS that the case is also present with "List (text)" field type.

🇫🇮Finland sokru

Updated the patch so that the PathautoBulkUpdateForm supports selecting bundles for other entities than nodes.

The MR has tests that pass, so removed the "needs tests" tag.

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

The MR is not supposed to be applied to 11.1.x but on 11.x branch. If this gets accepted to 11.x then we can create a MR for 11.1.x.

🇫🇮Finland sokru

Security support for Drupal core 10.2 has ended, but let's leave its removal for later time.

🇫🇮Finland sokru

Sorry it took so long! Merging this now and special thanks for the excellent steps to reproduce the issue.
I'll publish the change records.

🇫🇮Finland sokru

I guess it might be related by some faulty Search API configuration or source data issue, but on the other hand search_api_opensearch module has this change committed since day 0 https://git.drupalcode.org/project/search_api_opensearch/-/blob/3.x/src/...

🇫🇮Finland sokru

The issue was persistent on large Drupal 10.3.x project, however I'm not able to reproduce it anymore, so closing this. The original issue might need to be resolved on Drush or PHP memory configurations level.

🇫🇮Finland sokru

Would be benefical to have some tests for drush commands, but I'd leave it for another issue.

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

I've attached a tiny patch for core Claro theme to fix the issue, but IMHO it should be handled in core with BC etc.

So unless @dhruv.mittal has some better idea how to fix the issue, this I'll mark this Postponed on 🌱 Propose the Preview feature as an experimental Core feature Postponed . Feel free to create a corresponding core issue.

🇫🇮Finland sokru

Its read on /admin/config/content/same-page-preview on page load and stored to localStorage. https://git.drupalcode.org/project/same_page_preview/-/blob/2.1.x/js/adm...

I'd say the interpretation on issue summary is wrong and we should limit the scope of this issue just for the label copy.

Whatever label we choose it should be reflected to config schema on related issue.

🇫🇮Finland sokru

Added related (child) issue.

🇫🇮Finland sokru

+1 for RTBC.
I've created a separate issue I found when testing this issue 🐛 View mode does not persist on validation errors Active .

🇫🇮Finland sokru

@niharika.s Thanks for your effort, but did you test your patch with linked module? It needs more complete refactoring to work.

I've attached a patch for work-a-round for domain-module, but I'd say the fix needs to be worked on same_page_preview module.

🇫🇮Finland sokru

Added the upgrade path, tested manually on project with config:inspect command that configurations are valid now.

🇫🇮Finland sokru

On issue 📌 Add index to AlterSettingsEvent for backward compatibility Fixed there's some example code using EventSubscriberInterface, would that help you?
We probably should update the documentation ( https://www.drupal.org/docs/contributed-modules/elasticsearch-connector ) for better crafted examples.

🇫🇮Finland sokru

Created a MR that fixes the issue, but this module would greatly benefit for tests for this feature.

🇫🇮Finland sokru

This can be reproduced easily when creating a new content with Drupal core 11.x and 2.1.4/latest dev commit. I tried reverting 🐛 Preview only when editing content, not creating Active but it didn't help. Attached a screen recording.

🇫🇮Finland sokru

The patch contains proof-of-concept of the fix, it should be rather safe change.

🇫🇮Finland sokru

Thanks for the great module!
I didn't create a separate service to do requirement check like the main module is doing.
The submodule would also benefit for READMD.md or INSTALL.md help file, eg. exact composer steps how to download latest supported version.
Seems like https://github.com/josdejong/jsoneditor stopped including dist/ to codebase somewhere on 6.x.x.

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

This still needs work for issue mentioned on #12 (and #16). So it needs steps to reproduce, fix for that and tests to make sure it won't get regression.

🇫🇮Finland sokru

This needs tests. I'd recommend adding tests from https://www.drupal.org/files/issues/2023-10-05/2954352-diff-show-local-t...
and separate test for issue reported on #12.

🇫🇮Finland sokru

Looks awesome! Only thing I wonder if we should mark the template deprecated [#3385127] ?

No contrib modules seem to alter the template, but some custom projects might.

🇫🇮Finland sokru

For us the MR on related issue 🐛 AJAX MessageCommand markup and styling differs from Theme default Active does not fix the issue. We have a custom module with form_alter that places custom inline status message on specific place. After the 📌 Use MessagesCommand in BigPipe to remove special casing of the messages placeholder Fixed all status messages are rendered on same place:

This issue can be reproduced easily with core (11x.) by creating a custom module with following code and setting the site to maintenance mode and going to "/node/add/article".

<?php
use Drupal\Core\Form\FormStateInterface;

function MY_MODULE_form_node_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  // Some business logic.
  $form['field_example'] = [
    '#theme' => 'status_messages',
    '#status_headings' => [
      'error' => t('Error message'),
    ],
    '#include_fallback' => TRUE,
    '#message_list' => [
      'error' => [
        t('Failed to connect the API service, please try again later.'),
      ],
    ],
  ];
🇫🇮Finland sokru

Updated the issue summary for the issues that I found that should be finished before the beta/RC release. Maintainers might disagree with the list, but I'm sure they'd appreciate any help to complete any issue on issue tracker.

On general level the module has a lot of code inherited from Drupal 7 days. Also many issues would need someone to validate if the issue is still present and provide steps how to reproduce it.

🇫🇮Finland sokru

Unfortunately this breaks sorting when "Allow multiple selections" is selected. I'd recommend reverting the commit #54b5ef33 and adding the solution with tests.

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

I'll leave to maintainers to decide if they first want to complete the 🌱 Code modernisation Active .

🇫🇮Finland sokru

Needs work for field description. The failing tests are in process on several issues.

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

The tests on MR pass on Drupal 10, but fails on Drupal 11. Maintainers could either close this as duplicate for 📌 Drupal 11 compatibility Needs work , or merge this and handle Drupal 11 related tests on related issue.

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

The failing tests seems unrelated.

🇫🇮Finland sokru

I gave some thought on documentation page update, but I'd say its better to leave as simple as possible. The documentation page gives the instructions for "--help" flag for quick-start command, and that shows the instructions for all the options, including the password option.

🇫🇮Finland sokru

Required quite many restarts until the the unrelated test failures got passed status.

🇫🇮Finland sokru

Great work here @mparker17! I'll look into this in upcoming weeks.

🇫🇮Finland sokru

I fully support the idea, only thing I wonder is that many still uses outdated Drupal ( https://www.drupal.org/project/usage/drupal ) and its usually easier to upgrade contrib-modules before doing the core update. But maybe we leave 8.x-7.x branch for that process...

🇫🇮Finland sokru

sokru made their first commit to this issue’s fork.

🇫🇮Finland sokru

@murrow, you're correct there's currently no support for proxy connection.

🇫🇮Finland sokru

I think all threads should be resolved now.

🇫🇮Finland sokru

I'd recommend merging 📌 Add Gitlab CI to siteimprove module Active first, so we get extra assurance the this module is compatible with Drupal 11. So the now failing job "phpstan (next major)", should pass.

🇫🇮Finland sokru

I tried to keep the changes as minimal as possible, Eslint, Stylelint and CSPell issues should be handled on separate issue.

🇫🇮Finland sokru

I believe the correct status now is "Fixed".

🇫🇮Finland sokru

I believe the correct status is now "Fixed"

🇫🇮Finland sokru

@arunmark thanks for the merge request, however this still needs the upgrade path and the config updates, similar to what has been done on [PP-1] Make views exposed filter operator labels configurable Needs work .

🇫🇮Finland sokru

re: #46
Asking users to go to separate form to resolve this issue does not help the command-line use cases (CI pipelines, syncing database from different environment, etc.).

IMHO automatically creating the missing translation directory gives the best DX/UX. We could add some message to inform about autocreation.

🇫🇮Finland sokru

Related issue has tests and upgrade path that this is missing. On Views admin UI the patches from this issue uses the same label "Operator label" as [PP-1] Make views exposed filter operator labels configurable Needs work , either of them would need a change the label to avoid confusion.

🇫🇮Finland sokru

I don't think its postponed by anything, but needs a reroll.

🇫🇮Finland sokru

Removed the duplicate function that @quietone found, setting back to Needs review.

🇫🇮Finland sokru

Committed to 8.0.x, not sure if I find a time to fix this also on 8.x-7.x.

🇫🇮Finland sokru

I wrote the steps to reproduce the issue and created draft change record.

🇫🇮Finland sokru

@gapple Caching the nonce might be acceptable for someone, but tools like https://csp-evaluator.withgoogle.com/ or https://github.com/google/csp-evaluator for offline usage, will mark it as a violation: "This CSP contains static nonces. Static nonces allow attackers to bypass the CSP since they know the nonce value. Nonces should be cryptographically random."

Quote from https://content-security-policy.com/nonce/ under subtitle "Cons of using a Nonce vs a Hash":

The nonce needs to be generated dynamically, and must be different for each request

Netlify platform has CSP plugin, enabling it will result a random nonce value on each request. You can try by running curl -I https://app.netlify.com/ |grep 'content-security-policy' multiple times and nonce is different on each request.

🇫🇮Finland sokru

I thought I would be working on the failing tests, but after some reading on the subject I think the nonce should not be handled on application or backend level, becuase like @gapple mentioned on #2:

A nonce must be randomly generated per-response, not just per-site.

that's going to require disabling all the caches. This article describes an approach, where backend just provide a placeholder for nonce and proxy replaces the placeholder for each request.

Therefore I suggest we either close this issue as "won't fix" or re-title this for suggestion number 2 from issue summary "Implement a sha-X hash".

🇫🇮Finland sokru

+1 for adding this feature, so it would make easier to land the parent issue.
Left suggestion also to add H6 -level element.

I assume #18 was missing a cache clearance.

🇫🇮Finland sokru

I went through the remaining tasks and all but last issue was not done. I created a MR for remaining task.

🇫🇮Finland sokru

I assume usability review could think following issues:
- Position of lossless checkbox on form.
- Need for lossless checkbox on form. Since the IMG_WEBP_LOSSLESS equals to 101 value ( see: https://php.watch/versions/8.1/gd-webp-lossless). We could remove the checkbox and treat the 100 selection same as lossless selection (This is how Intervention/image does it)

Production build 0.71.5 2024