🇵🇭Philippines @abhaypai

Updating status to Needs Review after test run:

Need some advice or suggestion for following doubts.

1. Wondering why phpstan is throwing error elseif (!isset($value)) { i didnt change line for this and its giving error
2. Its weird that 1806 test run failed and at the same time it also shows core/tests/Drupal/Tests/Component/Utility/UrlHelperTest.php passed

Will create MR, once above issue is addressed.

Landing here from Bug smash initiative.

Few updates from end:

1. I am still able to replicate this issue on D11.
2. Added patch compatible with 11.x-dev version with interdiff from #57
3. Adding Needs Tests tag for help in solving failures in test
4. Did initial level of testing and works as expected.
5. Updated IS by adding sample for for Drupal 11

Landed here from Bug smash initiative.

+1 Patch is applied successfully for version 11.x-dev too and starting test for 11.x-dev version

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

Landed on this issue from https://www.drupal.org/project/drupal/issues/3227637 🐛 NodeController::revisionOverview is uncacheable Needs review and https://lendude.gitlab.io/bug-smash-initiative/ and i am still able to replicate this in D11

Might need some more insights here from the community, thats why moving it to Needs review status.

Is it designed that way or needs fix ? note i am aware of related issue https://www.drupal.org/project/drupal/issues/3232018 → and from my POV possibly we need to take some action on this issue, since it is affecting in many more pages with x-drupal-dynamic-cache: UNCACHEABLE in response headers.

Jumped in this issue from Bus Smash Initiative

Still replicating in D11.

Can we add a new js file naming checkbixunsaved.js, which will detect that if checkboxes are updated in a page or not ?
With this approach i believe we can detect and show consistent behaviour at least for this page.

Behavior can be defined as:

1. Observe if checkboxes are available in a page.
2. Keep the statuses in cached array as per element id, only on status change by user
3. Add or remove it from array, based on changes made by user
4. And then we can show message based on above user behavior

Came here from Bug smash initiative. I am still able to replicate this issue on D11 and tried initial level of debugging.

Wondering why CacheableMetadata::createFromRenderArray($username)->addCacheableDependency($node) or CacheableMetadata::createFromRenderArray($username) are not working as expected. Checked type of argument that i am passing and also looked into multiple other ways of defining it, still x-drupal-dynamic-cache is not coming on response headers.

Might need some more insights here from the community.

Thanks for looking into my suggestion and updating your comments on #2, although i have few points to share:

1. From where did you find the source of this following statement, Any reference link issue is gonna be helpful here to proceed with solution
   

   In order to avoid that _access_theme: 'TRUE' is avoided from block.admin_add route.

2. Can you please share replicating steps to land on this url /admin/structure/block/add/page_title_blockfrom user interface ? I aware of landing on this url directly from url path is possible and forms field shows default theme inputs.
3. Also i cannot see any code use case of this said /admin/structure/block/add/{placeholder} path or block.admin_add route source in the system without being usage of theme name to any pluginid

Looking for suggestions here from other community members; what can be solution of this bug ? Ideally from my POV form should not be visible if there is no theme available or installed in the system.

Keeping the status to "Needs work" since phpunit test is failing

Keeping the status to "Needs work" since phpunit test is failing, adding "Needs Test" tag and updating the version to "11.x-dev" since i also replicated the same in that version

Thanks @LiamMorland to report the issue.

Few updates from end so far:

1. I was able to replicate the issue according to the description
2. Did comparison between block.admin_library which defines path admin/structure/block/library/non_existance_theme and block.admin_add which defines path admin/structure/block/add/page_title_block/non_existance_theme have a delta of _access_theme in block.routing.yml file
3. Did some initial debugging and after applying _access_theme under block.admin_add issue is resolved

I believe we can close this ticket after reporter's or maintainer's confirmation looks like this is something close to and very similar to D7 open issue for [PP-1] Taxonomy autocomplete does not validate for term name length → and D11 Open issue for Entity reference does not validate auto-created entities 🐛 Entity reference does not validate auto-created entities Needs work

Moving this issue for Needs review before closing the ticket since it needs confirmation if we can reference this issue or not.

I will try to replicate the issue and work on proposed solution.

Updating the status to Needs work, i guess postponed was incorrect to select.

Thanks @viren18febs for sharing the fix as proposal.

Few inputs i wanna share here are :

1. #3 patch applied successfully via composer
2. Error is appearing in the settings because you haven't defined correct element for form element name
3. {TheRightElement} is just a placeholder which @NicklasMF has shared.
4. Please understand the logic of Default Favicon, Favicon Upload, or Favicon Path and then appropriately replace form element with {TheRightElement} in the code

Sharing screenshot for reference.
Also changing the status to Needs work and as a bug will need a test case to move the issue forward.

Thanks for reporting.

Will need steps to reproduce.

Also changing the priority to Normal and status to Postponed until reporter adds more information in the issue.

Thanks @omeya_yang for working and fixing the issue.

Patch #5 applied and tested manually also attaching image for retention of url. LGTM.

As a bug will need a test case to move the issue forward.

Thanks @msq for reporting this bug and also creating a patch for it.

I did replicated the issue as mentioned and just wanna share that point "Saving node A will result in 404 response" is not true, node is saved as expected but yes reference to deleted node still persist in the article which may result in 404 when viewing the article from user experience point of view.

I also applied patch for the same and confirm that patch is applied successfully with no issues and upon resaving or opening the article content i can see that "" is used to deleted content and also it is not visible in the frontend side.

Attaching screenshot for reference above mentioned points and changing status to needs review, so this issue can move forward with closure.

Thanks @omeya_yang for raising the issue and providing the patch.

I initially replicated the issue you mentioned and it is true that it is not saving the url am not sure if this is suppose to work like this which i am sure is a bug. Attaching screenshot for reference.

I also tested this patch with version you have shared, looks like there is one issue while saving due to undefined method "getLangcode".

Not sure if this is only for me so changing the status to "Needs work" and also attaching the screenshot for your reference.

abhaypai → changed the visibility of the branch 3250974-add-pakistan-urdu to active.

abhaypai → changed the visibility of the branch 3250974-add-pakistan-urdu to hidden.

For a bug report we need steps to reproduce the bug.

I believe this works as expected and proposed resolution is good solution for the time being according to the reporter although it is true that "Leave blank for all" is misleading according to my understanding if administrator doesnt wanna configure it via expose filter option.

Just sourcing above information based on my initial level of debugging and saw that "All" option or Default value only works with exposed filter as per this and this line number.

I will leave it up to the reviewer or reporter, if this issue can be closed (works as designed).