- Issue created by @pandaski
- Status changed to Needs work
over 1 year ago 9:54am 24 May 2023 - Status changed to Needs review
over 1 year ago 11:15am 25 May 2023 - last update
over 1 year ago Custom Commands Failed - 🇮🇳India shailja179 India
@pandaski,
This patch will fix the issue. Try this. - last update
over 1 year ago Patch Failed to Apply - 🇮🇳India shailja179 India
Uploaded wrong patch by mistake. This is the final patch.
Please use this. - Status changed to Needs work
over 1 year ago 4:49pm 25 May 2023 - 🇺🇸United States smustgrave
Thank you for reporting
Issue summary should be updated to include the proposed solution.
Also will need test case showing the bug.
Moved to 11.x as that's the current development branch also. Code will be committed there first to be backported.
- 🇮🇳India mrinalini9 New Delhi
Rerolled patch #5 for the 11.x branch, please review it.
Thanks!
- last update
over 1 year ago Patch Failed to Apply - 🇦🇺Australia acbramley
Found this issue because it can also cause a fatal if you pass a large number (which is interpretted as a float) into the offset.
TypeError: Cannot assign float to property Drupal\views\Plugin\views\query\QueryPluginBase::$offset of type int
The class prop was introduced here
IMO we also must update
setOffset
to strictly type on int as well since it'll just error otherwise anyway.We can then cast the offset to an integer when calling that function.
- last update
over 1 year ago 30,112 pass, 2 fail - @acbramley opened merge request.
- Status changed to Needs review
over 1 year ago 4:14am 5 September 2023 - last update
over 1 year ago 30,135 pass, 2 fail - last update
over 1 year ago 30,132 pass, 2 fail The last submitted patch, 13: 3361177-13-test-only.patch, failed testing. View results →
- last update
over 1 year ago 30,137 pass - Status changed to RTBC
over 1 year ago 4:41pm 5 September 2023 - 🇺🇸United States smustgrave
Applied the MR and searched for ->setOffset( and sees all instances were replaced.
Thanks for updating issue summary too! That helped.
- last update
over 1 year ago 30,137 pass - Status changed to Needs work
over 1 year ago 9:19am 8 September 2023 - 🇬🇧United Kingdom catch
I don't think we can add the type hint to a base class without a deprecation, this is running into 🌱 [Meta] Implement strict typing in existing code Active .
We can do something like comment out the argument (with the added type hint) then the debug classloader will start warning downstream code. There's an example for interfaces in https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... → but that should work equally for a base class.
In the meantime maybe we can implement strict type checking at runtime?
- 🇦🇺Australia acbramley
I don't think we can add the type hint to a base class without a deprecation
The prop is strictly typed to
int
, I would've thought passing a non-int would throw an error but looks like it actually doesn't! I guess cause we don't have strict_types = 1 on the file? - 🇬🇧United Kingdom catch
I guess cause we don't have strict_types = 1 on the file?
This is very likely, got caught out by that recently, without it anything that can be cast semi-properly will go through - so '0' vs false it doesn't care but false vs stdClass or similar it does.
- 🇺🇸United States alphex Atlanta, GA USA
Me as well, Drupal 10.1.5: Patch #8 solved it.
- 🇮🇳India abhinand gokhala k
Me as well, Drupal 10.1.5: Patch #8 solved it. :)
- 🇦🇺Australia acbramley
Created patch from MR For testing
Why do you need to create a patch from an MR to test it? Uploading patches to issues with in development merge requests makes extra noise on the issue that makes it harder for people to understand and review.
- 🇮🇳India Nishant2312
Hi all
For Drupal 10.1.5: Patch #8 solved my issue. :)
- 🇮🇳India abhinand gokhala k
Oh ya, that's right. That is my mistake. I tried to delete it but that was not possible.
- 🇺🇸United States bkosborne New Jersey, USA
Ran into this as well, but I just updated to view to have the offset set to 0
- last update
over 1 year ago 29,678 pass - 🇦🇹Austria tgoeg
For anyone who wants a quick but more or less proper fix without touching code for the time being:
Export your site's config, grep for "offset: null" in the view config .ymls and edit it to say "offset: 0" (or whatever valid value you want).
Reimport config. Site running again.
- 🇺🇸United States Rielrednot
#32 prompted me to look at the config and find all the spots where offset was null. I then went into those views (there was only 6) and set the offset to 0 there. No need to mess with the config yml directly.
- 🇬🇧United Kingdom attishu
Hi all
For Drupal 10.2.2: Patch #8 solved my issue. :)
- 🇺🇸United States rishi kulshreshtha
Suggestion given in comment #32 worked perfectly with Drupal 10.2.3. Thank you!
- 🇨🇦Canada benoit.borrel
For those wanting to apply the diffs from merge request 4707 (based against 11.x) on 10.2.x, here is the patch generated by cherry-picking the MR's commits against origin/10.2.x.
- 🇵🇱Poland lordzik
When this patch is going to be commited to Drupal 10.x?
I don't want to apply this manually after each Drupal 10 update... - 🇦🇺Australia digitalcatalyst
After upgrading core to 10.3.6, still seeing broken view offset error, applied patch from #41 to fix the issue.
- 🇳🇱Netherlands johnv
Setting the field to be required may be an easier trick to do the job.
See a similar problem with the 'Items per page' field: 🐛 Views Pager setting 'Items per page' with empty value gives WSOD Active - 🇳🇱Netherlands johnv
The issue 🐛 Implement constraints for Pager config Active contains a directive to use constraints, rather then if-then constructions.