- 🇺🇸United States smustgrave
This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.
As a bug this will need a test case
Not sure this is the correct fix though? Shouldn't we figure out what's calling this hook that leads to this? Could be wrong on that
But the proposed solution should be added to the IS.
- 🇦🇺Australia nterbogt
I think it comes down to whether we support NULL or "" in other yml config. If we do, this is a bug... if not, then it's just identifying a misconfiguration that it could cope with before.
- 🇺🇸United States smustgrave
When can getMediaQuery() return null for a breakpoint? Think once we know the steps to trigger that it will help I think.
More of these kind of tickets have been popping for 8.1
- 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
What if the breakpoint entity should handled the trimming in ::getMediaQuery and always returned a string?
And then we could add Breakpoint::hasMediaQuery instead of the empty, which is always a source of random issues.
- 🇫🇷France kae76 France
Thank you pameeela → on your comment [ #12 🐛 trim(): Passing null to parameter #1 ($string) of type string is deprecated in _responsive_image_build_source_attributes() Needs work ] worked a treat
- Assigned to PrabuEla
- Issue was unassigned.
- Status changed to Needs review
12 months ago 6:31pm 5 March 2024 - Status changed to Needs work
12 months ago 6:37pm 5 March 2024 - 🇺🇸United States smustgrave
#18 doesn't appear to be answered
Issue summary still needs to be updated
Tests appear to be missing.
- First commit to issue fork.
- Merge request !10561Resolve #3269845 "Passing null to parameter #1 ($string) of type string is deprecated in _responsive_image_build_source_attributes()" → (Open) created by vidorado
- 🇪🇸Spain vidorado Logroño (La Rioja)
I've decided to adopt the solution proposed by @larowlan in #18, as it makes sense to me. Additionally, I've added tests for
Breakpoint::getMediaQuery()
to ensure the behavior is properly covered. - 🇺🇸United States smustgrave
If that's the solution we are going to go with mind updating IS proposed solution section please.
- 🇺🇸United States smustgrave
Rebased to run test-only feature https://git.drupalcode.org/issue/drupal-3269845/-/jobs/3820945
which shows the coverage
Don't see any open threads in the comments or MR so believe this one is good.
- 🇳🇿New Zealand quietone
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 → .
- 🇪🇸Spain vidorado Logroño (La Rioja)
Sorry, I completely overlooked the second point in #18.
I've implemented it, covered it with a test, and also updated the issue title. I also took the opportunity to rebase the MR.