- 🇬🇧United Kingdom andrew.farquharson
Please review the new change record → .
- 🇺🇸United States vlyalko
Update, wanted to confirm that combination of the patch #12 from this issue: 3284807-12--fix_tag_based_caching_highlighted_fields.patch and https://www.drupal.org/files/issues/2024-08-21/3209441-23--cache_metadat... → fixed all the issues for me:
Excerpt is visible for anon user
Search highlighting is working as expected.Again, my setup:
Drupal 10.3.1
Search_api 8.x-1.35
Search_api_solr 4.3.5 - 🇺🇸United States vlyalko
Patch in #12 is not solving the issue for me either
And also confirm that Excerpt is not visible for anonymous users (using search_api_solr)
Using solr views and Search result excerpt in the Content type displayDrupal 10.3.1
Search_api 8.x-1.35
Search_api_solr 4.3.5 - First commit to issue fork.
- 🇫🇮Finland heikkiy Oulu
I can also report the patches seemed to fix the initial language issue after deployment but like mentioned in #23 the English labels made a return a bit later. We also discussed this further in Drupal Slack and there I mentioned that the returning of the English labels seemed to have something to do with cron. When I ran the cron tasks manually on the server it seemed to have an effect on the labels. So basically running drush cr command first to fix the labels and then a cron task again seemed to revert the labels back to English.
Unfortunately I haven't been able to track a specific set of commands to reproduce the issue. I guess this issue is so hard to fix because the problem can happen in several ways and steps of the workflow.
- 🇳🇱Netherlands SanderJP
I have seen the issues described here on a number of websites we maintain, where the translations are used correctly after a deploy. Some time later however the English defaults are back again.
The patch of #9 does seem to fix the broken translations after a cache rebuild. We will now test and see if the English defaults are still being reset after a while.
I'm surprised this Major core issue is hardly getting any attention at all. - First commit to issue fork.
- 🇺🇦Ukraine sickness29
This is already done and have test coverage on 2.0.x branch
- First commit to issue fork.
- 🇳🇱Netherlands Ruuds
The previous patch didn't apply to Drupal 10, so i've updated it.
- 🇮🇳India prem suthar Ahemdabad- Gujrat , Jodhpur - Rajsthan
Re-rolled The Patch #73 with The MR. Please review
- @prem-suthar opened merge request.
- 🇮🇳India prem suthar Ahemdabad- Gujrat , Jodhpur - Rajsthan
prem suthar → made their first commit to this issue’s fork.
- 🇮🇳India prem suthar Ahemdabad- Gujrat , Jodhpur - Rajsthan
Re-rolled The Patch #3 with The MR. Please review.
- @prem-suthar opened merge request.
- 🇮🇳India prem suthar Ahemdabad- Gujrat , Jodhpur - Rajsthan
prem suthar → made their first commit to this issue’s fork.
- 🇸🇰Slovakia poker10
@akalata By my comment about the "third thing" I meant that the MR is hashing the password to
$form_state['values']['pass']
, but the password is still saved to$account->password
in a plaintext. So I was not sure about the benefit of the solution, if we still keep the plaintext password on another place. I will try to take a look at this, what other possible solutions we have here. Thanks! - 🇺🇸United States smustgrave
Previously tagged for tests which still appear to be needed. thanks
- 🇺🇦Ukraine sickness29
I have added a functional test to test t=fr nested reference fields on node and paragraph inside that both use layout paragraphs.
Also adjusted method from base class to always use latest dialog title and save button in case we few dialogs deep. - @sickness29 opened merge request.
- 🇩🇪Germany mrshowerman Munich
The current MR produced a fatal error when using the LinkitFormatter:
ValueError: func_get_arg(): Argument #1 ($position) must be less than the number of the arguments passed to the currently executed function in func_get_arg() (line 24 of modules/contrib/linkit/src/Plugin/Linkit/Substitution/Canonical.php).
That's because the formatter does not pass
$options
, so I quickly added sanity checks to all occurrences offunc_get_arg(1)
.Apart from that, I agree with @berdir. I don't understand why we are passing the options to the substitution plugin that creates the URL, and still process the fragment and query parameters afterwards. I also think this processing code has a bug (left a comment on the MR).
- First commit to issue fork.
- @pbonnefoi opened merge request.
- @fskreuz opened merge request.
- 🇬🇧United Kingdom jessebaker
Reviewed, but further changes/refactor needed, thank you.
Rebased and applied suggestions from #30 🐛 Form state storage changes in #process callbacks are not cached during form rebuild. Needs review . Leaving in Needs Review for subsystem maintainer review.
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
Took a look at the code and agree with catch, seems reasonable - I could only find some nits, I've left them, but I don't think addressing them will advance the issue.
What we need here is some input from the Form API maintainers that there's no downsides to this change.
@effulgentsia is one of those and said this earlier
Especially considering that #process callbacks get run again after the form is retrieved from cache
, but I don't think that helps alleviate the issue. There are some genuine places where the only tool you have in your toolbox to alter a form is a process callback - examples of this would be e.g. as a plugin where you're building a plugin settings form but need to alter the larger form.
I'll ping Alex and Tim. Leaving at Needs review to reflect the actual status.
- 🇬🇧United Kingdom aaron.ferris
Added a couple of comments, is the !important necessary with the selectors as they are?
- 🇦🇹Austria drunken monkey Vienna, Austria
Would someone be willing/able to write a failing test for this? That would make it much easier to find the correct solution, I think.
-
ivnish →
committed a6533751 on 8.x-4.x authored by
robbiehobby →
Issue #2703229: [regression] Flags not appearing in node and comment...
-
ivnish →
committed a6533751 on 8.x-4.x authored by
robbiehobby →
- 🇨🇦Canada tame4tex
This has also broken any site that used
@current_record_count of @total
in a Results Summary when using a "Some" pager.I have opened 🌱 "Some" Pager Plugin and [view:total-rows] Regression Issue Active to address this.
- 🇯🇵Japan ptmkenny
After reading through this issue and 📌 OOP hooks using attributes and event dispatcher Needs review , as suggested in a comment above 📌 OOP hooks using attributes and event dispatcher Needs review , it seems that there are two different approaches:
- Using attributes for hooks as is done in the hux module →
- Using events alongside hooks as is done in the Hook Event Dispatcher module →
These approaches do not seem to be mutually exclusive.
In 📌 OOP hooks using attributes and event dispatcher Needs review , core has chosen to adopt the hux module approach.
This issue is about event dispatching as is done in the Hook Event Dispatcher module, whereas 📌 OOP hooks using attributes and event dispatcher Needs review is about using PHP attributes to replace hooks (the change record → states In a later Drupal version (Drupal 12 at earliest) we expect procedural hooks no longer to be supported and then these services and these LegacyHook marked shims will need to be removed.)
So if this issue is closed, my question is: has a decision been made to NOT support emitting events in addition to hooks? I originally started following this issue because I use Hook Event Dispatcher → , which currently has over 16,000 other users. It would be nice to have a little more clarity regarding the "duplicate" status here.
- 🇺🇸United States TolstoyDotCom L.A.
I had this issue on Drupal 11.0.4 and it was because there was no
language.entity.en
config entity. There were such entities for various other languages, just noten
. I have no idea how that happened.Just because it might mask an underlying issue is no reason not to make the
updateLockedLanguageWeights
method more bulletproof.I've attached my attempt to do that based on D11 dev of Oct 18, 2024.
- 🇨🇭Switzerland berdir Switzerland
> I have no clue whatsoever why the poll.tokens.inc file is not being recognised. Moving the code into .module works.
Strange, but tokens.inc will eventually go away anyway: 📌 Deprecate hook_hook_info() Active .
- 🇨🇭Switzerland berdir Switzerland
> this is effectively resolved in the next major version
Do we know *why* it was resolved there? 7.x doesn't have $options, so apparently there are other solutions that don't require this.
Even if it's fixed there, the test coverage should be ported to 7.x to verify that there.
This issue and patches here likely predate the change to return URL objects. Just like ✨ Allow "Linkit URL converter" filter to generate links based on the language of the current page, rather than the language of the referenced entity Postponed: needs info , where I made the same arguments, why are we passing in $options, set it on the URL object that we then return again? Can't we just set it on the returned URL object (while keeping BC for 6.x where it might not be a URL object yet)
That other issue moved away from this approach too, didn't test it yet, but apparently that worked there too, with a very similar requirement.
- 🇺🇸United States tr Cascadia
Yes I read it, I have been following it, and I don't see how it's a duplicate.
- 🇬🇧United Kingdom catch
@tr the other issue is 266 comments long, it also has multiple related issues, did you read it/them before re-opening this one?
If you didn't read it, then why contradict the people who have read it and worked on this issue for the past decade without doing any research?
- 🇺🇸United States tr Cascadia
Is this really a duplicate? I don't think it is.
This issue is NOT about replacing hooks with events, it is about having events dispatched in addition to hooks so that an event-based programming model can be supported. Is that really being done in that other issue? There's no mention of that in the change record.
- 🇺🇸United States rovo Delaware
#44 fixed it for me in 10.3.6.
Problem I experienced, a paginated view with Ajax, when clicking the paginations it would reload the View list, but stuck on page 1.
- 🇺🇸United States johnpicozzi Providence, RI
Ok, MR is now mergeable. Would be great to get this rolled in.
- 🇨🇦Canada teknocat
Further to this, it seems like it would also be prudent to have per media-type view access for any media types that use private files.
We have built a few sites now that need to have private files. Depending on the requirements for any given site, we might use a paragraph type to attach the files to a node, whereby that paragraph type has access restrictions - thus public users can see the node, but only logged in users see the paragraph. Other times we just have a field directly on a node and that node itself might be restricted view access.
So the tree-concept for access checks certainly makes sense, so that if the media entity was attached a field directly on a private node, or to a paragraph that's attached to a private node, then it's access would be restricted based on the root-level entity access.
However, there may still be cases where that doesn't necessarily work, depending on the individual use cases. So being able to set view access by the media type that uses private files would offer that first level of access restriction. I mean, what happens if you detach a media entity with a private file from it's parent entity that had the access restriction on it, but somebody out there still has a link? If the media entity itself is published and "View media" is allowed by anonymous users, they can download it.
Something we developed for a client was a document library, which provided a custom UI in the form of a Javascript Vue app that mimicked the functionality of a file desktop file explorer. This library widget is provided as a Drupal block and can then be added to a node using a block field in a paragraph, allowing the client to create a page with other content to go along with it as they see fit. We like to allow our clients to be able to manage all pages of their site the same way, add them to menus the same way etc to provide a consistent UX throughout the CMS. This doc library block and paragraph type is ONLY available to be added to a node type that is private, but the media entities themselves are in no way associated with the paragraph or the node. It's just a way to provide access to this fancy media browser, thus even if this tree-type access solution was in place it wouldn't work in this case.
This may well be an edge case, but if there were view permissions per private media type that was used to control download access to it's files, then it would be easy. Since we already had a custom module built to provide this custom block widget, we implemented a custom permission for document library access and then implemented the appropriate hooks for media view operation as well as file download with the logic needed to prevent access based on the file and media entity type and whether or not you have the permission.
The logic in the download hook is a bit messy, but it works.
Automatically closed - issue fixed for 2 weeks with no activity.
-
mparker17 →
committed e384e786 on 3.x
Issue #3480990 by mparker17: Add automated tests
-
mparker17 →
committed e384e786 on 3.x
- 🇨🇦Canada mparker17 UTC-4
Created ✨ Add database updates test Active and ✨ Add tests for views component Active
- Issue created by @mparker17
- @useernamee opened merge request.
- 🇺🇸United States johnpicozzi Providence, RI
- 🇨🇦Canada mparker17 UTC-4
I think it might be worth splitting the remaining tests into separate issues, and merging this one, so I'm going to do that.
- 🇺🇸United States rovo Delaware
#31 solved errors for me on Drupal 10.3.6.
Error:
TypeError: Illegal offset type in Drupal\Core\Entity\EntityStorageBase->getFromStaticCache() (line 183 of /var/www/html/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php).
and Error:
Warning: array_flip(): Can only flip string and integer values, entry skipped in Drupal\Core\Entity\EntityStorageBase->loadMultiple() (line 278 of /var/www/html/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php).
- 🇬🇧United Kingdom catch
Moving to duplicate just so people don't think there was a commit here.
- 🇺🇸United States nicxvan
This is fixed in 📌 OOP hooks using attributes and event dispatcher Needs review :D
- 🇺🇸United States mark_fullmer Tucson
I think you can only do this in the 6.1.x to provide a warning that a new param will be added in the next major:
Based on my testing in comment #50 and confirmation from another in #51, this is effectively resolved in the next major version. The intent in the recent merge here was to provide equivalent functionality for folks who still cannot upgrade to Linkit 7.x due to API changes introduced there.
Thanks @casey for providing the MR that does not include the second parameter to
getUrl()
. Folks who are interested in this change in 6.1.x, please review! Hi,
Able to reproduce the issue in the Drupal11.x. After creating the view with specific html element data is not coming for the iframe tag.MR !4595 has been verified in the Drupal 11.x.
Before Patch:
After Patch:
- 🇳🇱Netherlands casey
We already updated to 6.1.5 :(
Created a new MR containing the suggested change of kim.pepper
- @casey opened merge request.
- @casey opened merge request.
- 🇮🇳India atul_ghate
I am not able to reproduce this issue on Drupal 11.x. The steps I followed to reproduce are as below:
1.Clone Drupal 11.x and enable the Language and Content Translation modules.
2.Change the Manage Display settings for the media type and select the Thumbnail formatter with the Link to entity option enabled.
3.Create a Media type (image) and enable translation for it. Add an image with an English translation, then add a translation for another
language for the same media image
4.Add this media image to an Article node.
5.When I click on the image, it redirects me to the correct language version of the node.
Please let me know if there is anything I am missing. - 🇺🇸United States rhovland Oregon
Test failure seems to be unrelated to this issue
- @rhovland opened merge request.
- 🇮🇳India bgogoi
I am facing almost similar issue - can not add any new field to any content type:
he website encountered an unexpected error. Try again later.
TypeError: Cannot access offset of type Drupal\Core\StringTranslation\TranslatableMarkup in isset or empty in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 45 of core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php).
Drupal\Core\Plugin\DefaultPluginManager->getDefinition() (Line: 16)
Drupal\Core\Plugin\Factory\ContainerFactory->createInstance() (Line: 76)
Drupal\Component\Plugin\PluginManagerBase->createInstance() (Line: 136)
Drupal\field_ui\Form\FieldStorageAddForm->processFieldDefinitions() (Line: 80)
Drupal\field_ui\Form\FieldStorageAddForm->buildForm()
call_user_func_array() (Line: 528)
Drupal\Core\Form\FormBuilder->retrieveForm() (Line: 279)
Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
Drupal\Core\Controller\FormController->getContentResult()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 593)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 709)
Drupal\Core\DrupalKernel->handle() (Line: 19) - 🇳🇿New Zealand quietone
I re-tested today with Umami on 11.x and was able to reproduce the problem. I can only assume that my earlier testing was incorrect.
@manpreet_singh, thanks for the fix. Drupal uses peer review for all changes to core, so the setting on an issue to RTBC should be done by another contributor. Thanks.
This issue will also need a test to prove the fix works and expected, adding tag.
- 🇨🇦Canada mparker17 UTC-4
Merged the Message Listing test into the CRUD message tests; it kinda makes more sense there.
Added Create/Read/Update/Delete Message-Type tests.
The CRUD message-type tests briefly broke because of 🐛 Feedback message type entity doesn't declare its success_message property Active , but after merging that, they are working again.
- 🇦🇺Australia kim.pepper 🏄♂️🇦🇺Sydney, Australia
I think you can only do this in the 6.1.x to provide a warning that a new param will be added in the next major:
public function getUrl(EntityInterface $entity /*, array $options = [] */);
- 🇺🇸United States smustgrave
So this issue has been open 9+ years.
No one on 8.x or 2.x branches seem to asking for it, so I think I'm going to close. But if needed think it would be good to start a new ticket so that those who worked on this ticket can receive their credit.
- @valic opened merge request.
- 🇺🇸United States mark_fullmer Tucson
Okay, release 6.1.6, published on 2024-10-17, reverts this change: https://www.drupal.org/project/linkit/releases/6.1.6 →
Setting this back to "Needs work" for an implementation that avoids BC-breaking changes.
- 49576baf committed on 6.1.x
Revert "Issue #3022261 by sleitner, ckaotik, jurgenhaas, mrinalini9,...
- 49576baf committed on 6.1.x
- 🇺🇸United States mark_fullmer Tucson
IMHO this should be reverted, or a follow-up should be done to remove the argument from the interface.
Re-opening this issue, with the plan to revert the change so that the 6.1.x branch does not introduce BC breaking changes...
- 🇨🇭Switzerland berdir Switzerland
Found the issue: ✨ Allow "Linkit URL converter" filter to generate links based on the language of the current page, rather than the language of the referenced entity Postponed: needs info .
- 🇨🇭Switzerland berdir Switzerland
This issue introduced a breaking change:
public function getUrl(EntityInterface $entity, array $options = []);
As a result, media_entity_download for example has a fatal error again with this.
This was also proposed in another issue where I had already provided feedback about that.
IMHO this should be reverted, or a follow-up should be done to remove the argument from the interface.
- 🇺🇸United States smustgrave
Issue summary appears to be missing standard templates so tagging for such
have not reviewed
- 🇧🇪Belgium BramDriesen Belgium 🇧🇪
Thanks @berdir!
in hook_tokens() make sure you only run logic/calculations when required. For example, it currently seems to load all votes and calculates things, but the tokens for that might not actually be used. Even then, that logic seems extremely expensive (looping over all votes twice?)
Also noticed that. But wanted to get the tests in place before I started changing stuff logic wise :-).
I have no clue whatsoever why the poll.tokens.inc file is not being recognised. Moving the code into .module works.
Made the change so that it only affects the views module rather than globally allow
iframes
in the admin.Some further thought is probably necessary for whether this exposes any new potential security vectors.
e.g. if the site builder is using a user supplied token like a query string as a filter. But I'd assume as long as the value isn't rendered as
{{ token|raw }}
, it should be properly escaped or better yet, they should be using an appropriate filter in the first place like{{ token|escape('uri') }}
.codebymikey → changed the visibility of the branch 2942327-views-strips-out to hidden.
I have created MR with the fix for both themes please review.
Before
After
- @saurav-drupal-dev opened merge request.
-
bramdriesen →
committed 719c86b9 on 2.0.x
Issue #2913992 by bramdriesen, vegantriathlete, sja112, berdir, ivnish:...
-
bramdriesen →
committed 719c86b9 on 2.0.x
- ivnish Kazakhstan
I tested it manually using steps for reproduce. MR fixing the issue
- 🇮🇹Italy giordy
- Drupal 10.3.6
- Linkit 7.0.0-alpha2
- CKEditor Anchor Link 3.0.0-beta1I have the same problem as #52: when I insert the anchor it is deleted.
- 🇧🇪Belgium BramDriesen Belgium 🇧🇪
Not entirely sure #63 was handled. There is still quite a lot of if elseif else nesting being added here which increases the code complexity/readability.
- 🇧🇪Belgium BramDriesen Belgium 🇧🇪
Re #10
Finally, we'll probably also want to do something similar for Poll Choice. Perhaps that should be a separate issue?
I don't think this is needed. I tested a bit and I highly doubt anyone would want to remove the translation of a choice in that way. Currently the choices are kept in sync between the source and the translations, which I think is what we want! You don't want to end up with 3 choices in one language and 4 in another.
One thing I noticed, which needs a follow up issue is that the prompts to delete a vote or choice are not being shown. Your vote and option are just being deleted without the warning being shown (tested on 2.0.x without the change in this issue).
- First commit to issue fork.
- 🇺🇸United States liberatr Portland, OR
#95 wasn't applying for me, attempted a re-roll.
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇬🇧United Kingdom scott_euser
Is it worth considering removing the update hook and drush command altogether into a follow-up? Ie:
- Without the update dates entered before 7.27 would be no worse off
- Events created that are that old probably exact time matters little by now
- Workarounds would then exist because of the new options available
It would make the merge easier right?
- @bramdriesen opened merge request.
- 🇩🇪Germany a.dmitriiev
search_api_excerpt
extra field has cache contexturl.query_args
. I assume it is not taken into account when Search API Cache plugin for views is in place. The plugin collects object cache tags withgetRowCacheTags
method, but cache context is not taken into account. Maybe there is need to implement `alterCacheMetadata` method and add the cache context? But in this case the plugin needs some how to collect this cacheable metadata from the rendered entities that havesearch_api_excerpt
extra field for this view. - 🇩🇪Germany Anybody Porta Westfalica
@thetwentyseven would you mind creating the final fix as MR? And still needs tests.
- Issue created by @useernamee
- 🇮🇹Italy trickfun
Hi,
i can't apply patch 2896155-6 to 4.0.0-alpha4 and 4-devThank you
- 🇨🇦Canada mparker17 UTC-4
Have a test of basic functionality in place.
Updated the list of remaining tasks to include other tests we need.
- @mparker17 opened merge request.
Automatically closed - issue fixed for 2 weeks with no activity.
- 🇨🇦Canada mparker17 UTC-4
I have a test drafted but still need to type it and work out bugs; assigning to myself.
- Issue created by @mparker17