I just double-checked this and there is no change between 11.2 and the 11.x branch except that the TypeError is thrown. BigPipe JS also can't find a placeholder in a <noscript> tag in 11.2 - I had never actually noticed this fact before, since it wasn't a problem.. :)
I would assume that if something has already been replaced server side, then it would not be sent to the browser, so would not be a target at all.
Yes that would be correct. The exception is only thrown in the browser when it was not done server side.
There is no way to tell big pipe "only do this replacement server side because it's in a noscript tag", but such complexity isn't needed. We can just keep things working like they did historically: don't throw if the browser can't find a placeholder.
Actually I realized my attempted work-around I mentioned in #6 doesn't work, because the noscript tag is added to the dom, and whatever is inside actually loads. So, I think my MR is the best solution: bigpipe javascript should just ignore any situation where target is null.
Markup in a noscript tag is by definition already special-cased, because it's not interpreted as being part of the dom. And since it's only relevant when javascript is disabled, handling replacement on server side is fine.
I don't think there anything special that bigpipe should do, except just not error in any rare cases where the target can't be found.
I think this would mean that no other placeholder strategy would find the placeholder either.
Not sure what you mean by the - the non-javascript placeholdering has always worked fine - the placeholders are replaced on server side.
I did find a work-around I could use on Drupal 11.3 and later: I can instead have the noscript tag be part of the replacement markup, instead of wrapping the placeholder. Then, of course the javascript can find the placeholder, because it's not in a noscript tag.
However, rather than having to work around the issue, I think it would be best to avoid throwing here - just don't try to process the target if it can't be found.
It does not seem a matter of checking the value of file_size in the template file, but rather to know which formatter is using that template file, or use two different template files.
See the following paragraph of the issue summary - this should be handled by 🐛 'file_table' formatter shows file size twice Active (a new size toggle available to formatters or other callers).
I don't see a need to update the template docs because the with_size variable isn't used there. (It /could/ have been used there, if we moved the logic from the preprocess function into the template, but I didn't put the logic there to minimize the need for updating templates.) I assume templates don't need to exhaustively document all possible variables.
Do we need to add a code comment in TableFormatter where we set #with_size to FALSE?
Never mind, moved the fix here in hopes of faster reviews
I incorporated a fix for this Claro bug in a MR for 🐛 'file_table' formatter shows file size twice Active , which resolves the related issue re: file module needing logic to turn off file size display when it's already displayed elsewhere.
While fixing this issue w/ file module, it seemed like a good time to also fix a related issue with claro module, which, unlike the default file link template, was the missing logic to not show the file size if it's empty.
The views filter is now released in version ^3.2, ^2.1 and ^1.12
unfortunately, since the table changed between 1.x and 2.x I don't think the same view can be used across all versions - but maybe the view could be altered on 1.x using a hook
I noticed that it recurses through .git directories too - that wouldn't be as bad as node_modules but, still seems like a waste
@carlitus this bug was fixed here in hreflang module, and was released in version 1.15
Oh cool, well, not sure how much time I might have to literally maintain this module - but the good news is, it should be quick and easy for us / someone to build the view and add it to this module.
Actually, I think it would be useful if we allow the filter to be exposed, so that the end user can turn the duplicate filter on/off? Updating issue summary to say so.
Since the parent issue was cherry-picked to 10.6.x this should as well?
@fishfree I wasn't able to reproduce this exception in my quick testing.
Does it happen for you on a fresh, clean install of Drupal core?
What version of Drupal core are you using?
Do you have any contrib modules installed?
If I'm not able to reproduce it then I won't be able to work on a fix...
I think this should also be query a optimization when there are tens of thousands of files - it can jump to the next fid rather than scanning through a large row offset.
Not sure if this was officially released but in any case I added a link
Thanks for reporting!
Already resolved by humans
It looks like the sessions table was removed from system module in Drupal 10.3.0, according to https://www.drupal.org/node/3431286 → - it's now created on demand by the session handler.
So yes, we now need to check if the table exists. This also means that the code that gets a description of the sessions table from system schema (for use on the config form) only works on older versions of Drupal.
What session handler do you have in use? Ideally, we could provide ip_anon support for removing IP addresses from alternative session storage
Opened 🐛 Actually add service alias for Symfony\Component\EventDispatcher\EventDispatcherInterface Active to add the alias correctly
mfb → created an issue.
Removing the ability for loggers to depend on config would be a big trade-off for the contrib loggers that I maintain - there would no longer be a nice, friendly UI for admins to tweak logging without having to tinker with the settings file or yaml files. It certainly would help resolve circular dependency hell, though...
@farse on the current 7.x branch I am still seeing a ServiceCircularReferenceException if I install symfony_mailer module and core workspaces_ui module along with any logger that uses config, such as core syslog module or Raven module.
Given that the issue also affects core syslog module, I would say the issue should ideally be resolved in either symfony_mailer or core?
There might be some workaround we could do in Raven module, but I'm not sure off hand what that would be, aside from no longer using config..
For future reference, this ideally would've been filed as a new issue, as it was a new bug in the 7.x branch, but in any case thanks for reporting
Please test the MR - I believe this should resolve the issue by refactoring config to "properly" override (rather than having a special snowflake config service).
For anyone seeing a circular reference, please provide steps to reproduce from a fresh install of Drupal core. This would include which contrib module(s) and how they need to be configured to reproduce the bug. (If a custom module is required to reproduce the bug, please upload or link to some example reproduce code.)
For extra bonus points you could open a merge request with a failing test :)
@dfisher I still don't know how to do this. I may have missed something obvious as I don't use this module per se, was just trying to make my contrib module compatible with it.
Move feature request to current dev branch.
I believe you would now need to call something like Sentry.addIntegration(integration: Sentry.replayIntegration());, as we now use a callback function for drupalSettings.raven.options.integrations rather than a simple array.
Move feature request to current dev branch
Move feature request to current dev branch
Move feature request to current dev branch
In a followup issue we may need to add a log filtering feature? Although, developers can do their own filtering via Sentry callbacks.
For example, maybe by default, logs could honor the same "ignored channels" and "ignored messages" that errors do, with a checkbox to capture all logs, including ignored channels and ignored messages? There might even be a use case for logs having its own lists of ignored channels and ignored messages.
Actually fixed on the 7.x branch
Actually fixed on the 7.x branch
Actually fixed on the 7.x branch
mfb → changed the visibility of the branch 3541436-backport-3 to active.
mfb → changed the visibility of the branch 3541436-backport-3 to hidden.
This likely needs a backport
I discovered an API method we can use to convert the description markup to plain text: PlainTextOutput::renderFromHtml(). And for some reason, the default title is empty string, but the default description is null. So, we need to handle that potential null by only using the description if it's present.
Opened up a MR for review. Since title is apparently used in de-duplicating items, i thought it might be a good idea to save the truncated description instead of an empty title? But also, since a NULL or empty title would be valid (if saved programmatically, for example), I also patched the label() to use the truncated description, if necessary. Since the description can be HTML-encoded markup, we have to strip tags and HTML-decode the description before using it as the title, otherwise the markup would be displayed to the end user.
Setting to needs work because this isn't really fixed by the patch, yet :)
I updated the issue summary with a pretty-important remaining task: Figure out exactly how to handle RSS items that are missing a title or have an empty title, due to the item title being used in the UI, e.g. when listing items, and by the label() method; all this stuff still needs to work for items without a title!
Far as I can tell you're good here, the dependency is on ^2.17, which allows any later 2.* version to be installed, including the current version, which is 2.24.0
Tests are failing, but I don't think it's related to this MR, as tests are also failing on the 2.x-dev branch
So I think this could be RTBC?
Added a commit to resolve a deprecation notice I've been hitting on a production site:
Deprecated function: Drupal\aggregator\ZfExtensionManagerSfContainer::setContainer(): Implicitly marking parameter $container as nullable is deprecated, the explicit nullable type must be used instead
As far as sustainability a few things come to mind:
- A file hash submodule could report duplicate files and even allow them to be merged.
- A "sustainability" contrib module could instruct site admins to install helpful modules and configure them more sustainably.
I don't intend to enable the dedupe validator by default. It is a complex feature that could be enabled site-wide, on a per-field basis, or programmatically; there are also the choices of "strict" dedupe and "original" dedupe. And I do consider it a "proof of concept" - sites may want to improve the dedupe UX.
@liam morland did you see my comments in #30? I feel like it would be helpful if this issue could decide to either trigger a deprecation if null is returned in cases where it is not documented to be possible, or change the documentation to allow null, despite the data model not allowing null.
In other words, my @todo was something to figure out in this issue, not a future issue.
We didn't make another issue AFAIK. The current state of the patch is "for review" to figure out next steps.