#Needs Review Queue Initiative

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

Even for simple issues such as this one, leave the standard issue template in place. It can be unpredictable what is discovered while working on an issue and having the template already in place provides a place to keep track of anything that pops up.

This seems pretty straightforward and seems helpful also for CMS.

I converted the patch to an MR.

Please ignore #161 and use this patch instead. It is identical to #69 except for one line which makes it compatible with Drupal 10.4

Hi all, I was using the patch at #69 with Drupal 10.1, and when I upgraded to Drupal 10.4, the patch at #69 no longer applies. I tried the patch at #158, which does apply to Drupal 10.4, but in my usecase does not fix the issue.

I am including herein a version of the patch at #69 which now applies to 10.4, in case it might be of use to anyone.

Added phpcs exclude like the the referenced case.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

@florianmuellerch thanks,

I have tried to reproduce what you said in the issue description:

In this particular example, I'm having translatable nodes with a paragraph field on them. Each paragraph has a "published" flag which itself is translatable. The goal was if I have a translation of the node but don't want to show one of the paragraphs there, I could simply unpublish it and it won't get shown on the translation.

What actually happens is that another language of the same paragraph which itself is published (coming from another translation of the same node) is being shown.

I have installed a new Drupal 11.1.0 site and then did the following:

* drush site:install -y
* composer require drupal/paragraphs
* create a module mymodule which implements function mymodule_language_fallback_candidates_locale_lookup_alter(array &$candidates, array $context) { $candidates = []; }
* drush en -y paragraphs language content_translation mymodule
* /admin/config/regional/language/add
* Language nane: French
* Click "Add language"
* /admin/structure/types/manage/page
* In Language Settings, select "Enable translation"
* Save
* /admin/structure/paragraphs_type/add
* Label: test paragraph
* Save and manage fields
* Create a new field
* Select "Plain text" and continue
* Label: "Field inside paragraph"
* Selct "Text (plain)"
* Continue
* Save settings
* /fr/admin/structure/paragraphs_type/test_paragraph/form-display
* Put "Published" in the visible section and save
* /admin/structure/types/manage/page/fields/add-field
* Select "Paragraphs" and continue
* Label: Paragraph
* Check "Test paragraph"
* Save
* /admin/config/regional/content-language
* Check "Paragraphs"
* Unfold the Paragraphs section
* Check the "test paragraph" and its sub-fields, "Published" and "Authored on"
* Save configuration
* /node/add/page
* Title: "page in English"
* In "Field inside paragraph" write "Hello world"
* save
* /fr/node/1/translations/add/en/fr
* Title: "Page in French"
* In the paragraph, uncheck Published
* Save (this translation)
* visit /fr/node/1 in an incognito window
* confirm you see "Hello world" even though the paragraph is unpublished in the French translation
* curl -O https://www.drupal.org/files/issues/2022-11-10/3186034_10_respect_empty_... →
* patch -p1 < 3186034_10_respect_empty_translation_candidates.patch
* drush cr
* visit /fr/node/1 in an incognito window
* based on the issue description, I would expect *not* to see "Hello world", but I see it.

I will do some more tests but for now I cannot reproduce a scenario where my site acts differently with this patch than without it.

I read the IS, MR and the comments.

The suggestion by @larowlan in #18 has two parts and one has been implemented. The part to "add Breakpoint::hasMediaQuery instead of the empty, which is always a source of random issues" is not done and I see no comments about that.

Also tagging for a title update. The title should be a description of what is being fixed or improved. The title is used as the git commit message so it should be meaningful and concise. See List of issue fields → .

@alberto56 I expect this behavior as you wrote only if the paragraphs field itself is not translatable (Paragraph fields officially do not support that). I assume that if it‘s translatable, the two values (fr and fr-fr) are no longer connected.

Maybe also try with another language like de or en (just more standard that fr-fr). But otherwise your suggestion should sound right (except without the patch, the fr-Version should be shown in fr-fr to be precise, because fr-fr is unpublished which then falls back to fr).

I have inherited a project which uses this patch, and I'm trying to figure out what this patch does exactly.

I have created a node in language fr and a translation in language fr-fr.

The node in fr has a paragraph test, which is published; and the paragraph is translated in fr-fr so that it's not published.

From reading this patch description, I would expect:

* That with the patch, the paragraph test does not appear in fr-fr
* That without the patch, the paragraph test appears in fr-fr even though it is unpublished in fr-fr (put published in fr).

However, whether or not I apply this patch, the paragraph does not appear in fr-fr.

In fact I cannot see any difference anywhere on my website whether I apply this patch or not.

If anyone can advise me on a step by step guide to reproducing a scenario where the site will act differently with and without this patch, when starting from a brand new Drupal site, that would be very appreciated!

Patch is currently the same as the MR

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

We (@benjifisher, @godotislate, @mikelutz, and @quietone, with some input from @larowlan) discussed this issue on Slack (https://drupal.slack.com/archives/C226VLXBP/p1736647603190789).

Despite Comment #60, we agreed to convert annotations to attributes first, and then to deal with source_module, so I am postponing this issue on 📌 Convert MigrateSource plugin discovery to attributes Active . Once that issue is done, we will have to completely rewrite the MR here, but I think that will not be hard. We have already done the work of deciding what has to be done, and all we will have to do is re-implement it.

When I am feeling really optimistic, I think that if we can get both issues done before 11.2.0 is released, then we can skip the deprecation step. If source_module never exists as an attribute in a release of Drupal core, do we have to deprecate it?

Fixed broken images in issue summary. For some reason the did not save previously.

Fixed by 🐛 Prevent saving config entities when configuration overrides are applied Needs work

Lets try again, updated to 11.x and fixed tests <3

bbrala → changed the visibility of the branch 3104408-allow-to-include to hidden.

bbrala → closed merge request !1308

MR rerolled on 11.x
Patch attached for composer

duaelfr → changed the visibility of the branch 3326950-expose-imageitem-width to hidden.

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

@smustgrave, could you explain better what do you mean by backwards compatibility and in which case should we trigger an error?

Feedback in MR seems legit.

Wonder if we have to worry about backwards compatibility and add a trigger_error

The changes need to be applied to the merge request.

Thanks for this !

Attach is the patch from #48 (2875033-46.patch) rerolled for Drupal 10.3.x and 10.4.x

Cool!! Sounds good!
I'm going to take a look around, ask a few colleagues as well and report back when I have more tangible elements.

Cheers!

It should be done as upload progress needs revival but not clear about " the modern approach"

Thanks Andrey!

So this ticket is a Won't fix?

Or to be moved with external lib in contrib?

Any recommendations on what to do with this ticket? How to move it forward?

removal is targeted to D12 📌 Remove jQuery Form dependency from misc/ajax.js Needs work

but this issue needs new approach

I can confirm that also the MR https://git.drupalcode.org/project/drupal/-/merge_requests/10896.diff solves the error regarding "Drupal\Core\Database\TransactionNameNonUniqueException" on group relationship migrations.

Thanks Andrey (@andypost)!

Found the corresponding CR: core/jquery.form library deprecated → as of 9.4.0

Found the internal.jquery.form library:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/core.librarie...

Is the jQuery.form uploadProgress getting removed from Core in D11?

Any suggestions on the changes that would be needed ?

Thanks in advance!

Any reason to use jQuery.form which is deprecated for a long time?

Yeah only bad code interpreting the feed could be a danger here, because untrusted third party strings need to be just that, untrusted, and run thru something like HTML Purifier or the filter built into drupal core; i.e. security issues shouldn't be possible regardless of what's in a third party string.

Coming here from 🐛 Double encoded ampersand in /admin/reports/updates Active which I was pinged about in Slack for a subsystem maintainer review.

From a little Git archeology, looks like this double-encoding was introduced in #1003764: Add an alter hook invoked while generating release-history XML files → when @drumm converted this from manually putting the XML together into using SimpleXML. Apparently no one noticed that SimpleXML is doing another round of encoding for us.

I just did this:

% wget https://updates.drupal.org/release-history/save_edit/current -O save_edit.xml
...
HTTP request sent, awaiting response... 200 OK
Length: 9702 (9.5K) [text/xml]
Saving to: ‘save_edit.xml’

save_edit.xml             100%[=====================================>]   9.47K  --.-KB/s    in 0s

2025-01-15 14:45:23 (23.8 MB/s) - ‘save_edit.xml’ saved [9702/9702]
% grep title save_edit.xml
<title>Save &amp;amp; Edit</title>

So the source is definitely double-encoded. It seems wonky to say that core should have to decode twice to get the raw values.

I don't understand the "BC" concern here. The primary client for these feeds, update.module, was written to assume properly encoded markup in the XML feeds, and is now "broken" by the double encoding. If other clients are already decoding twice, and we stop double encoding, what's the harm?

Here's a little test script:

$inputs = [
  'not_encoded' => '<title>Save & Edit</title>',
  'encoded_once' => '<title>Save &amp; Edit</title>',
  'encoded_twice' => '<title>Save &amp;amp; Edit</title>',
];

foreach ($inputs as $key => $value) {
  echo "------\n$key\n";
  echo "Raw: $value\n";
  $decoded = html_entity_decode($value, ENT_QUOTES | ENT_HTML5, 'UTF-8');
  echo "Decoded: $decoded\n";
  echo "Double decoded: " . html_entity_decode($decoded, ENT_QUOTES | ENT_HTML5, 'UTF-8') . "\n";
}

If I run this, I get:

------
not_encoded
Raw: <title>Save & Edit</title>
Decoded: <title>Save & Edit</title>
Double decoded: <title>Save & Edit</title>
------
encoded_once
Raw: <title>Save &amp; Edit</title>
Decoded: <title>Save & Edit</title>
Double decoded: <title>Save & Edit</title>
------
encoded_twice
Raw: <title>Save &amp;amp; Edit</title>
Decoded: <title>Save &amp; Edit</title>
Double decoded: <title>Save & Edit</title>

We're still going to turn around and re-encode this when printing it in the available updates report. That happens via core/modules/update/templates/update-project-status.html.twig right here:

<div class="project-update__title">
  {%- if url -%}
    <a href="{{ url }}">{{ title }}</a>
  {%- else -%}
    {{ title }}
  {%- endif %}
  {{ existing_version }}
  {% if install_type == 'dev' and datestamp %}
    <span class="project-update__version-date">({{ datestamp }})</span>
  {% endif %}
</div>

So Twig is going to escape this for us, and I don't see how this could be used as an attack vector.

IMHO, this is the right fix, not the extra decode being proposed at #3493742. I am a maintainer for project*, so I could just commit this, but I can't deploy the fix on d.o anymore. 😅

Initial attempt at reformatting issue summary with standard template.

Issue summary needs help filling a few sections in particular:

• Remaining tasks
• User interface changes
• API changes

Adding screenshot from #79.

dydave → changed the visibility of the branch 9.5.x to hidden.

dydave → changed the visibility of the branch 2833968-upload-progress-using to hidden.

So issue summary seems incomplete/incorrect. Should follow standard issue template please.

There are multiple MRs up, there should be 1. Others should be closed or hidden.

Was previously tagged for tests which still appear to be needed (if reviewing correct MR).

I added before/after screenshots to the issue summary and created a draft CR.

catch → closed merge request !10309

Committed/pushed to 11.x, thanks!

@mradcliffe: Both merge requests have been rebased.

Issue summary still needs to be updated.

Thanks!

https://git.drupalcode.org/project/drupal/-/merge_requests/10859.diff solved this issue for me after installing entity_reference_integrity on Drupal 10.3.10.

https://git.drupalcode.org/project/drupal/-/merge_requests/10896.diff also worked!

• Pipeline is now green.
• All feedback addressed.
• All threads resolved.
• Title and summary are accurate and clear.
• CR is simple, accurate and clear.

I see nothing left to improve. RTBC.

Thanks!
-Derek

p.s. I did some minor edits and formatting cleanups on the CR: https://www.drupal.org/node/3489411/revisions/view/13800107/13851975 →

Sorry, I missed something in previous reviews (or this is a new bug). 1 suggestion to apply, otherwise seems RTBC to me.

Yeah I'm working on baseline, the deprecated function was moved to views.module

Related, but the rebase seems to have clobbered views.views.inc entirely. I thought we still needed a deprecated views_entity_field_label() in there.

TL;DR: Probably wise not to self-RTBC after a rebase, especially with merge conflicts, so that peer review can help spot trouble. 😅

phpstan is failing in the pipeline.

Rebased thanks!

Working on this.

This looks good but needs a rebase.

The merge request 10896 (#45) seems to be 1770 commits behind the target branch and needs a rebase. It's currently failing tests.

The issue is a little hard to follow and I think next if someone could address the Needs issue summary update tag that would be helpful to clarify proposed resolutions and remaining tasks.

Ok thanks @mdsohaib4242

We're using the latest bootstrap version 3.34 and do not need this patch. I checked and I'm not using it.

Thanks a lot Elim (@elimw)! Great job!

We've just given a round of tests with the merge request you created MR!10896 and it fixed the issue as well for us 🥳
We've updated the patch in our project to use the one from #54.

@vistree:
Yes indeed, the patch created by Elim can be tested here:
https://git.drupalcode.org/project/drupal/-/merge_requests/10896.diff

It should fix the issue in your project as well.

It's a different (better) way of writing the same thing as the previous patch.

Thanks again for the work Elim and in advance for your feedback @vistree!

The error might be caused by a bug in version 8.x-3.25.
Update the Bootstrap theme to the latest stable version

composer require drupal/bootstrap:^3.26