Hmm interesting, that's something new in D11.1 then I guess, since I know the tests passed in D11.0 - we can probably can get rid of that alter anyway, I'll double check during christmas holidays
Thanks! Not sure why the tests currently fail, but that's not related to this patch.
swentel → made their first commit to this issue’s fork.
I've also added some info to the README file with a link to https://www.drupal.org/docs/drupal-apis/layout-api → as well.
Layout are based on core layouts. I suggest opening ds.layouts.yml to see many examples used in DS.
So requiring 4.3.0 seems to work fine, although I have some test failures locally on my own site, but I'll have to double check. Will rewind my local drupal install to 10.x to see if that goes better (and hopefully find some time to upgrade this module to work with 11.x too heh)
I need this one as well, works for me!
That's not an issue at all
The update hook was not to break things unexpectedly when upgrading DS on existing sites since the code was potentially a breaking change at the time this was committed. Even though the impact is relatively small, it would be a subtle bug which you wouldn't necessarily see immediately.
I'm planning more fundamental changes in code regarding templates etc, but it's good practice to always ship with a BC layer for upgrades -even for our own projects at work :)
Afaict it should be fine, the double hyphen is particularly necessary for the hook, the others are fine normally - even though we could have been consistent, that's something else :). Otherwise I would have seen this already I think since that issue is in 10.1 already
Aggregator won't work at all on D11 though, so no need for pipelines to run on D11 for now
swentel → created an issue. See original summary → .
can't test on holiday, but I fear it will, but can't tell for sure.
Note: bonus take for my original patch is that you can can make a setting that also allows to keep the original buttons as well.
Ok, thanks for the information! There might be a bug in core that fails to create the actual entity display maybe, however, we shouldn't fail on that of course. I have a different approach in my mind which I'll fix tomorrow.
Hmm, this would mean that this line would return nothing.
$row = \Drupal::config($name)->get();
Can you tell me the machine name of the vocabulary? Or maybe additional view modes that you have created?
I didn't really enjoy the error message as it was a blank page :)
Addded to the summary, thanks!
Cool! Committed to the project.
It's a nice feature, but using unset on the form structure generally isn't a good idea. Whether it's DS, or contrib module or a custom module doing form alter doesn't really matter.
Patch attached is a cleaner and safer option while preserving the sticky action buttons.
Setting right version
Back to gin as I need to know what this 'beta' function does. I'm searching for a key in the form structure ($form['actions']['submit']['#submit']) which doesn't seem to exist anymore? So is there an 'unset' happening or something? If that's the case, I'm not sure if that's the right approach to alter it like that, and not too keen on adding a check on gin admin theme.
smustgrave → credited swentel → .
You can completely ignore the 5.0.x branch, everything still happens on 8.x-3.x :)
Patch makes sense I guess. To be sure: which Drupal core version is this on, D11 of D10?
Moving to 8.x branch (other two are not supported) - will check soon, had no idea this was added.
Changing title as it makes more sense I think
Thinking about it some more the next day,
- maybe 'automatic authorization' isn't necessarily needed for this to reproduce. Didn't test it yet, but if it's not enabled, the code in the else statement calling validateAuthorizeRequest() would fail and then immediately return a response, where it it should probably be able to actually connect.
- My proposal for the fix could probably be easier as there's already the same 3 lines of code in the anonymous check. Maybe everything could
if ($bridgeRequest->get('client_id')) {
$_SESSION['oauth2_server_authorize'] = $bridgeRequest;
}
if ($this->currentUser()->isAnonymous()) {
$url = new Url('user.login', [], ['query' => ['destination' => Url::fromRoute('oauth2_server.authorize')->toString()]]);
$url->setAbsolute(TRUE);
return new RedirectResponse($url->toString());
}
This means though that $_SESSION will always contain the 'oauth2_server_authorize' which might not be necessary at all. But that would also be the case in my original proposal, so maybe that one could be changed a bit like this:
if ($this->currentUser()->isAnonymous()) {
// This part stays the same
}
else {
// A user may be authenticated at this point (e.g. registration flow ..)
if ($bridgeRequest->get('client_id') && isset($_SESSION['oauth2_server_authorize'])) {
$_SESSION['oauth2_server_authorize'] = $bridgeRequest;
}
}
Feedback welcome of course :)
Ok, this has been done already in 📌 Automated Drupal 11 compatibility fixes for field_group - Fixed PHPUnit on GitlabCI RTBC , sorry for the noise.
Ok, I'll be honest, I'm a bit lost at what exactly I'm looking at, but at some point I started looking at modules which have dependencies on modernizr and field_group still has one for horizontal tabs. Now, given that Modernizr is deprecated in 10.3 (this project runs on 10.3.0) and is removed in 11, I tried removing the references and everything started working.
I've opened 📌 Remove dependency on Modernizr Active to start removing that. The part which confuses me is that everything is fine when using Claro as admin theme, but I'm lost while debugging, and I'm fine fixing it in field group :)
Not sure if it's a mismatch or not, but the call to an aggregated JS URL (which is identical for the webmaster whether it fails or not) throws the 'Invalid filename. in Drupal\system\Controller\AssetControllerBase->getGroup() (lne 225)) exception as webmaster, but not anymore after reloading, AFTER I reloaded as user 1. Fun right? :)
If I can set a breakpoint somewhere to compare things, let me know, I can try and dig further then!
Looks like the modernizer 'fix' was just a red herring. It's becoming weirder if I revert the weight change because I now have a scenario locally where I can consistently break and fix the node edit screen (gin admin theme with gin toolbar, field groups)
- clear cache
- load node/x/edit as user 1: everything is fine
- load node/x/edit as webmaster: everything is fine
- clear cache
- load node/x/edit as webmaster: js broken (drupalsettings not found at all)
- load node/x/edit as user 1: everything ok
- load node/x/edit as webmaster again: everything is fine! (WUT!)
It's crazy. I suspected 🐛 Library information alter should not be context-aware Closed: outdated at first, but returning FALSE in _gin_toolbar_gin_is_active() as a test doesn't do the tricky, so now looking at gin theme itself
@catch : I spoke to soon (again), weirdly enough, the fix works on my local installation, but not on production, so back to debugging :)
Follow up: changing the weight of modernizer in 10.3.x fixes (as in #15) it for us as well, but also not sure if this is the 'right' fix :)
Bitten by this too - and I thought 🐛 Logic error in Drupal's lazy load for asset aggregation Active was the actual fix, but it ain't (yet). Reverting fixes some things for us (in our case, gin admin theme which breaks stuff in some cases for webmasters on node edit forms where the JS completely broken) .. will try to figure out if I can create a patch for gin - hints welcome as I'm not to familiar yet with the aggregation system)
Bitten by this too - and I thought 🐛 Logic error in Drupal's lazy load for asset aggregation Active was the actual fix, but it ain't (yet). Reverting fixes some things for us (in our case, gin admin theme which breaks stuff in some cases for webmasters on node edit forms where the JS completely broken) .. will try to figure out if I can create a patch for gin - hints welcome as I'm not to familiar yet with the aggregation system)
Probably related, but not sure 📌 Replace custom weights with dependencies in library declarations; introduce "before" and "after" for conditional ordering Needs work
Actually, I spoke to fast, while node edit page is now fine with the patch, admin/content is now broken, so it looks like the sort isn't solving everything.
We've been hit by this one as well. In our case, it happens while using gin admin theme, which also has two JS files with preprocess set to FALSE, and only under a specific case being a webmaster role which has less permissions than say user 1 (so hard to reproduce as well, as I'm not totally known with the internal system)
The JS aggregation URL triggers following error: Symfony\Component\HttpKernel\Exception\BadRequestHttpException: Invalid filename. in Drupal\system\Controller\AssetControllerBase->getGroup() (line 225)
The MR in #4 fixes it for us, and the page now renders fine, so plus one from me at least.
It's close to being critical, because in our case the webmasters, while they don't get a blank screen, they couldn't edit their content as for instance CKeditor isn't loaded at all or use the media browser to select images.
Merged and new release
new release available - did some quick manual testing and looks fine as far as I can see, tests seem to be passing as well (although the pipelines don't give me full reporting). If there are other problems, let me know of course !
Also, it's a task right :)
Changing title - will create a fork soon so I can push and see the pipelines running
This changes the info files, not sure (yet) if anything else is needed at the moment.
moving to right branch
Indeed, this is a logical use case and not related to the linked entity. Question is whether paragraphs is responsible for this behavior or not, as I can image core entity reference might have the same problem. You could easily render a view mode of a node which does not have any 'value' in an optional text field, I imagine this triggers the same problem.
I'll have a look this week! I'm going to add an option at /admin/structure/ds/settings that allows you to enable this option so that sites who upgrade don't have undesired consequences, even though the impact is probably minimal anyway, but you never know :)
Could you, just to be sure I can rule a module out, give me the list of enabled modules? I've had weird things conflicting in some weird way sometimes :)
Ok, so 10.2.x needs PHP 8.1 to run anyway, so this looks good. I might way until 10.3 comes out before committing this one to give people at least a bit of time to upgrade.
I'm confused simply because MicrosubChannelStorage doesn't even exist ... at least in the latest version of this module. which version of this module are you running? If so, make sure you upgrade the module as well.
Unless I'm doing something wrong, the patch doesn't apply at all anymore - also, there's an existing .gitlab-ci file already in the project, so the fork shouldn't contain that.
The weird thing is, this doesn't crash for me at all, on any content type here, so not sure how I can reproduce this. I wonder if there's another module installed on your install that might clash here?
This is more of a CSS thing which we don't ship with, sorry about that.
In this case, it's probably going to be hard to getting floating anyway as there's no wrapper for the value of the field either.
Better check in place now
My check is actually wrong
makes sense, committed and pushed!
ok, committed and pushed.
This fixes the failing context test
I think I have it! Are you able to test this patch? I'll be testing it in many places to make sure I haven't broken anything as this is tricky stuff :)
The interesting thing is, if you configure, and then save, everything is fine, also during rendering, it's Field UI though where something goes wrong, still digging :)
merged, thanks!
Hmm you're absolutely right, I can easily reproduce as well. It seems to work for other field though, but not description, looking into it!
committed and pushed, will tag a new release in a couple of minutes, thanks all!
Updated patch. Also checks if the field has a 'value' property because this doesn't necessarily has to a 'link' field.
Ooh, I see! You've configured Reply link to use the 'path' value, which isn't a 'link' field.
I guess there's confusion here, which is totally the fault of my UI :)
The context here of 'Reply link', is a URL to which your current node/content is replying to.
I'm guessing you configured thinking that this should contain the url of the current node, which it shouldn't.
So, quick fix to get rid of the notice: don't set a value for this one, unless you actually have a field on your content type which contains the absolute url of a post (either on your own site, or external).
However: let's make sure this is a bit clearer, I see 2 things
- document the properties of AP and what they should/can contain and why they are used. Most properties are obvious, but this clearly is confusing
- check if the field uses 'uri' or 'value' - I was assuming this would always be 'link' field, but this can also just be a textfield which stores a link
ok cool, could you also show me a screenshot of the 'note' type (probably at /admin/config/services/activitypub/activitypub-type/note ?)
converted to kernel test as the function didn't exist in the phpunit test
You're right, it's confusing, especially since whitelist defaults to TRUE in config.
I'd go to actually block the urls if nothing is configured in the textarea. We have to add an upgrade path for existing sites where, in many cases, the checkbox will be toggled probably without additional hosts. In that case, the upgrade path needs to turn of the checkbox.