#Bug Smash Initiative

mdranove → changed the visibility of the branch 3115978-required-summary-field-config to hidden.

I believe that in most of the cases where this error shows up in my logs, the client has made a GET request to /contextual/render. It looks like bots that are monitoring all the URLs accessed by a page request and trying to hit all of them. So they make a normal request to some page on the site, which triggers an ajax event that does a POST to /contextual/render, and somehow they see that URL and try to follow it.

I think it would be preferable to just silently fail in that case, versus the BadRequestHttpException. I certainly get a ton of these errors and they're effectively meaningless.

Hello this came up as a daily BSI target, since it's been a few years wonder if still experiencing this in D11?

Also tried testing on a standard profile install and could not reproduce.

Since it's been 3+ months since #9 without a follow up going to close out, but if still experiencing this in D11 please reopen.

Thanks all

For some of these values, it would also be super helpful to have a UI for that. In my current case for the fragment: ✨ Allow entering a fragment attribute Active

Fixed the problem in MR11697 and tested it here in tugboat. Please review.

Reverted the commit and marked 🐛 Symfony\Polyfill\Intl\Icu\Collator::compare() is not implemented Active as duplicate. Let's fix that here and re-commit.

I think we should also open an upstream bug report against Symfony to implement the method, then we wouldn't need the checks added by that MR and could go back to the original code committed here.

Just seen 🐛 Symfony\Polyfill\Intl\Icu\Collator::compare() is not implemented Active let's revert this and recommit with that fixed.

::compare requires a Collator to be passed to it as well so we'll need to allow that the handle it being NULL?

I missed the symfony polyfill in the MR though which does look like it works

This blows up sites that don't have the intl extension

adam@8fa2a3a71304:data $ drush cr
PHP Fatal error:  Uncaught Error: Class "Collator" not found in /data/app/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php:239

I think the intention of this code was to check the extension was loaded before initialising the $collator but I can't quite follow:

$collator = \Collator::create((!extension_loaded('intl')) ? ('en') : (\Drupal::service('language_manager')->getCurrentLanguage()->getId()));

Is this worth spending time on given this will swap to the generic revision UI in 📌 Switch Node revision UI to generic UI Needs review ?

I also noticed other weird behaviour when testing this such as the "Current revision" on the second page shows the revision log message of a different revision.

Rebased the MR that got previously reviewed in #111, #112
Also adding a patch file for cweagans/composer-patches usage

As #112 mentioned, there are a few outstanding questions left

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

Automatically closed - issue fixed for 2 weeks with no activity.

Since there hasn't been a follow up going to close out, if still an issue in D11 please reopen

Thanks all!

Automatically closed - issue fixed for 2 weeks with no activity.

catch → closed merge request !6869

I don't think there's anything left to do here, RTBC queue has been very busy (hard to keep under 100 issues even with over 100 commits/month and many more reviews).

Committed/pushed to 11.x, thanks!

Rebased. What else needs to be done before the merge?

Unfortunately this needs another rebase, the MR looks good to me.

Moving this to the Action module

I'm surprised it hasn't been mentioned here, but changing a Node author is locked behind the administer nodes permission (see NodeAccessControlHandler::checkFieldAccess) so checking node edit access is not enough.

Rebased and slightly simplified the solution. Also expanded the test comment a bit.

Definitely looks like a duplicate of 🐛 Access cacheability is not correct when "view own unpublished content" is in use Needs work , please reopen if not.

Drupal 9 is End of Life.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies → .

Seems like a simple change, IS is clear

1) Drupal\Tests\node\Functional\NodeEditFormTest::testNodeAuthorDisplayName
Behat\Mink\Exception\ExpectationException: The string "<em>3</em>" was not found anywhere in the HTML response of the current page.
/builds/issue/drupal-3183509/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-3183509/vendor/behat/mink/src/WebAssert.php:363
/builds/issue/drupal-3183509/core/tests/Drupal/Tests/WebAssert.php:559
/builds/issue/drupal-3183509/core/modules/node/tests/src/Functional/NodeEditFormTest.php:320
FAILURES!
Tests: 4, Assertions: 105, Failures: 1.

Shows the test coverage

See no issue.

My plan is to move the test plugins out of the field_test module so that we don't have to depend on that anymore...

After evaluating how much work that would take I ended up putting all of the entity_test module stuff in the fixture. It's a pain, but it worked without having to refactor code to work without it.

This is going to fail tests because I excluded all the entity_test lines from the fixture that I created. But I'm out of time to work on this right now and need to save the work. My plan is to move the test plugins out of the field_test module so that we don't have to depend on that anymore, reducing the amount of garbage that has to be in the fixture.

So this came up as a daily BSI target.

I tried out the code supplied in the issue summary and confirmed the password is saved as ' ', so this still appears to be valid.

smustgrave → credited griffynh → .

smustgrave → credited larowlan → .

@larowlan and @griffynh for 🐛 SessionManager improperly abstracts Symfony's Session Component Postponed: needs info

Im not entirely sure the current approach in MR 5004 is the right approach.

Started a new branch. I think it makes sense to just set this to not be required if it's the field config form. Only issue is there's logic setting the display to none by default for the summary default value. Think only thing left is to show that in JS on click of the checkbox. Then add a test.

Yes minified describes the library, not what to do with it.

@osopolar that’s a really good point — I should know better than to make assumptions about how this works. I was under the impression minified was an action to be taken rather than a descriptor. But you’re right, the docs clearly state otherwise. Looks like I’ve been shooting myself in the foot here! Thanks so much for pointing it out.

That said, I still think the page-cached asset URLs are a problem, regardless of my incorrect understanding. I’ll try to update the issue summary to reflect that.

This is still reproducible on 11.x

I don't really understand why this is happening in the first place, nor why the fix fixes it, we should definitely add some comments if that really is the proper fix.

dcam → changed the visibility of the branch 2865710-dependencies-from-only to hidden.

dcam → changed the visibility of the branch 11.x to hidden.

And if this MR went in, and you never added aliases, then they'd never get indexed at all.

True, but really, how often does this scenario happen -- a web site without aliases, yet with an urgent need for indexing? It seems to me highly theoretical.

Not having node/100 pages indexed would be fine in most cases, and probably preferred. It was a case of premature indexing in my case, and I did not want pages with node/100 indexed. Also, how does Drupal CMS shipping with robotstxt module affect this change? As I still see it, in the majority of cases, you do not want node/110 pages indexed -- As I wrote about this earlier:

And if that's true, we should make the preferred behaviour the default, non?

I still see this change as largely beneficial in the grand picture, for the majority of use cases. Conversely, if someone REALLY wants node/100 pages indexed, they can easily install the robotstxt module, and correct this.

I have the same errors in the log and now I am wondering how to fix it.

@joelpittet You mentioned that you had assets that were type: external AND minified: true and so you removed the minimized there. We have one such asset, the better_exposed_filters → library. But I am wondering why minified: true shouldn't be set for external libraries. There is a example of both attributes set in documentation → under minified, with the description

Whether the asset is already minified. Default: false

The asset in your example (https://cdn.ubc.ca/clf/7.0.4/css/ubc-clf-full-bw.min.css) seems to be already minified, so why not setting minified: true?

Not all of those views specified the content of the text, so there was nothing to update in those cases. But I updated the ones that needed a dependency.

Issue Summary

I tested the BREAK functionality in Drupal 11 using the latest patch from the 3017548-d11 branch with @jeff chicoine. The expected behavior is that content before BREAK should appear in the teaser view, while the full content should be visible in the full node view. However, my testing indicates that

is not working as expected.

Steps to Reproduce

1.) Set up a Drupal 11 environment using DDEV and Composer.

2.) Checked out the 3017548-d11 branch and pulled the latest changes.

3.) Created a new content type: Testing Text WYSIWYG.
Added two fields:
Basic Formatted w/ Summary (Basic HTML text format)
Full Formatted w/ Summary (Full HTML text format)
4.) Configured the Manage Display settings:
Both fields are set to show in Teaser and Full content display modes.
Teaser display mode set to "Trimmed or Summary".
5.) Created new content using this content type.
Added two paragraphs of text in both fields.
Inserted BREAK between the paragraphs.
6.) Viewed the node in both Full content and Teaser view.

Observed Behavior

Results of grep -rn "plugin_id: text$" --include views.view.*.yml:

core/profiles/demo_umami/config/install/views.view.frontpage.yml:303:          plugin_id: text
core/profiles/demo_umami/config/install/views.view.frontpage.yml:373:          plugin_id: text
core/modules/content_translation/tests/modules/content_translation_test_views/test_views/views.view.test_entity_translations_link.yml:104:          plugin_id: text
core/modules/views/tests/fixtures/update/views.view.test_filter_format_dependencies.yml:164:          plugin_id: text
core/modules/views/tests/fixtures/update/views.view.test_entity_id_argument_update.yml:193:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_display_empty.yml:34:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_display_empty.yml:40:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_entity_id_argument.yml:160:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_tokens.yml:109:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_tokens.yml:141:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_destroy.yml:50:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_destroy.yml:56:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_destroy.yml:148:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_destroy.yml:154:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_destroy.yml:161:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_destroy.yml:167:          plugin_id: text
core/modules/views/tests/modules/views_test_config/test_views/views.view.test_token_view.yml:205:          plugin_id: text
core/modules/views_ui/tests/modules/views_ui_test/config/install/views.view.sa_contrib_2013_035.yml:178:          plugin_id: text

Seems like a good update

1) Drupal\Tests\node\Functional\NodeTypeTest::testNodeTypeEditing
Behat\Mink\Exception\ExpectationException: The string "<em>Lorem ipsum.</em>" was not found anywhere in the HTML response of the current page.
/builds/issue/drupal-2978818/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-2978818/vendor/behat/mink/src/WebAssert.php:363
/builds/issue/drupal-2978818/core/tests/Drupal/Tests/WebAssert.php:559
/builds/issue/drupal-2978818/core/modules/node/tests/src/Functional/NodeTypeTest.php:160
FAILURES!
Tests: 6, Assertions: 95, Failures: 1.
Exiting with EXIT_CODE=1

Shows the test coverage

Great extension of an existing test.

LGTM.

This is a "closed (won't fix)" for me.

I created this issue because /node/100 style paths got indexed, possibly before they had aliases.

And if this MR went in, and you never added aliases, then they'd never get indexed at all. rel="canonical" should cover this even when an node goes from unaliased to aliased.

Drupal CMS ships with robotstxt module now so I feel like 'default behaviour' is covered there, but we can't just break search indexing in core because people usually install contrib modules.

1) Drupal\Tests\node\Functional\NodeRevisionsUiTest::testNodeRevisionsCacheability
Behat\Mink\Exception\ExpectationException: Current response header "X-Drupal-Dynamic-Cache" is "UNCACHEABLE (poor cacheability)", but "MISS" expected.
/builds/issue/drupal-3227637/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-3227637/vendor/behat/mink/src/WebAssert.php:180
/builds/issue/drupal-3227637/core/tests/Drupal/Tests/WebAssert.php:969
/builds/issue/drupal-3227637/core/modules/node/tests/src/Functional/NodeRevisionsUiTest.php:229
FAILURES!
Tests: 5, Assertions: 52, Failures: 1.

Shows test coverage

Change seems straight forward and gone through a number of reviews. Believe feedback has been addressed.

Probably need to update any default views that ship with core to include these. Example views.view.frontpage.yml in node, there may be others.

Feedback appears to be addressed.

I applied the suggestion and rebased the MR too.

I followed the same steps as listed in #20 and I can reproduce the error #Atlanta2025

hmm the new test that fails is the one that's being added so something is still wrong here.

There are more tests that are failing on the last test run. All in ChainedFastBackend.

The change however makes sense and it has good test coverage.

small code improvement needed

the wording need a little bit more work, let's try to get a native, documentation-inclined person to help out

The test only job didn't fail, but reverting the fix causes the new test to fail - here's the failing job:

https://git.drupalcode.org/project/drupal/-/jobs/4790389

I converted the test-only patch to an MR, it passes locally, maybe it will fail with 100 runs.

The < check is still in core and I don't think it would hurt at all to change that to <=

📌 Optimize last_write_timestamp writes in ChainedFastBackend Active means that when there is one cache write on a page, the two servers would have to be more out of sync for there to be a problem, but it's only 50ms which is will within NTP tolerance, so it only makes the problem less likely and doesn't eliminate it. I think therre might be an issue around to try to use something else (like a special cache tag) that doesn't rely on clock time.

I have exactly the same issue as described in #20.

Turned out that when content moderation is enabled for a content type and there are multiple translations, the table node_field_data will list only published translations. This is a problem because nid relationship has extra condition to match the language

For me the solution was quite obvious

/**
 * Implements hook_views_data_alter().
 */
function example_views_data_alter(array &$data): void {
  // We need to remove this extra language field in order to make it work
  // with unpublished translations of moderated content that are not listed in
  // node_field_data table.
  if (isset($data['node_field_revision']['nid']['relationship']['extra'])) {
    unset($data['node_field_revision']['nid']['relationship']['extra']);
  }
}

Is this still relevant since 📌 Optimize last_write_timestamp writes in ChainedFastBackend Active ?

I messed up that MR while trying to update it for 11.x. I'll sort it out tomorrow.

Hi @jwilson3,

The original merge request seemed to be abandoned and the title was still incorrect.
Since there was no update, I created a new branch using cherry-pick and opened a new MR with the correct title as suggested.

Some minor adjustments were also made to fix issues that were causing test failures.

Let me know if there's anything that needs to be changed or improved.

Is the button tag going in? See #40 and #43 above.

Patch in #80 worked well for me. I needed video and source tags and this patch enabled them. Thanks!

Adding a tag.

@norman.lol :

In which class do you propose adding this method?

Since TrustedCallbacks → , sth like this does not work anymore:

    $element['#pre_render'][] = [HelpCallbacks::class, 'preRenderHelp'];

Fix is:

    $element['#pre_render'][] = HelpCallbacks::class . '::preRenderHelp';

mohit_aghera → changed the visibility of the branch 11.x to hidden.

mohit_aghera → changed the visibility of the branch 2637808-languageformatter to hidden.