🇺🇸United States @brad.bulger

Yes, this looks like it's working. I did a new install of Drupal 10 with ggroup 3.0.x and saw the original problem, then installed the issue fork branch. It only shows the types of subgroups that installed on the group type, and when there is only one it defaults to the form for it.

Domain Source isn't dependent on Domain Access (at least it isn't anymore). I separated the changes made for the fields of each module so that it will work for either or both.

There's an additional unrelated problem, that I have to fix to do anything about this issue. In DomainMicrositePathProcessor::processOutbound(), there is a line:

$domain_source = array_shift(array_column($entity->get('field_domain_source')->getValue(), 'target_id'));

The argument for array_shift() is passed by reference, so it has to be a variable. The array_column() call has to be stored and then passed to array_shift().

$target_ids = array_column($entity->get('field_domain_source')->getValue(), 'target_id');
$domain_source = array_shift($target_ids);

brad.bulger → created an issue.

OK. This may be a duplicate of some existing API issue, I'll keep looking.

Yes, either 📌 Information is missing from the API pages Active is a duplicate of this, or I need to find/create another issue for API, or it's not an API issue (seems unlikely).

I don't know if it is strictly about interfaces. We would have to find a class that extends another class and overwrites a parent class function not defined in the interface. I have done some searching but that's tricky to find.

I don't quite understand what this issue is reporting, but it looks like {@inheritdoc} is not working. Child class method pages don't show the annotations from the parent class method. Is that this issue?

I wondered about that, how would I know which is the case? The copy of the API at api.druphelp.com would imply that the code itself is fine, I'd guess. ContentEntityBase::get() is {@inheritdoc} and the parent is FieldableEntityInterface::get() and that has all of the relevant information. So is this an API module processing bug?

brad.bulger → created an issue.

fwiw, we have a few dozen domains each with their own menu on our site, and we had to change route_param_key to a longblob - some values are over 10K.

I couldn't figure out how to do it, sorry.

brad.bulger → created an issue.

I hope I did that properly, I'm still new to using issue forks instead of patches.

brad.bulger → created an issue.

brad.bulger → created an issue.

I think I know why you had that absolute URL logic in there. In the unpatched code, links to nodes that are tied to other domains with Domain Source show correct URLs to that domain. With the patch, they all come out in the current microsite domain. These are links generated by Linkit in CKEditor 5.

brad.bulger → created an issue.

May be related to this other long standing issue.

brad.bulger → created an issue.

Maybe. That issue seems oriented towards trying to prevent the problem. I am thinking more of how to document it. I am just not sure if there is a way to describe what's happening. Maybe not in 100% of the cases, but would "don't load an entity in this hook" be accurate?

brad.bulger → created an issue.

+1 on adding this. I needed to wrap the included view output in custom HTML that included values from other fields in the outer view, but not if the included view output is empty. The only way to do that is to do the custom HTML in the view field. This patch applied to 1.0.0-beta5 with only some whitespace warnings.

brad.bulger → created an issue.

I'm running Drupal 9.5 (9.5.12-dev ) and Drush 11.7.0-dev. Domain 2.0.x-dev. Still see this behavior.

Can users detect the E_NOTICE from an unserialize() error with an error handler? That wouldn't mean that they are now, of course.

@cmlara I suppose that it is unlikely that the comment would be updated now while a code change that moots it is being considered. That would be the only reason to split the bug off, I think.

The issue is to spell out that any non-string scalar values will be stored as strings. Something like

mixed $value: The value to store. Non-scalar values are serialized automatically. Scalar values are stored as strings.

I don't doubt that this is as designed, the issue is to make it explicit.

Actually using the real service demonstrates this easily:

> $userData = \Drupal::service('user.data');
= Drupal\user\UserData {#9804}

> $userData->set('test', 1, 'testbool', FALSE);
= null

> $userData->set('test', 1, 'testint', 3);
= null

> $userData->set('test', 1, 'testfloat', (10/3));
= null

> $b = $userData->get('test', 1, 'testbool');
= ""

> $i = $userData->get('test', 1, 'testint');
= "3"

> $f = $userData->get('test', 1, 'testfloat');
= "3.3333333333333"

> var_dump(compact('b','i','f'));

array(3) {
  'b' =>
  string(0) ""
  'i' =>
  string(1) "3"
  'f' =>
  string(15) "3.3333333333333"
}
= null

If there are other places where values are handled in the same way, then yes, it would be helpful to note this there as well.

The core form doesn't because it's the whole browser page, so submitting a form gives you the normal feedback of a POST - the spinning wheel of internet delay etc. I get why it seems like a feature request, but I would never give this to my users without something to tell them "yes the form was submitted, please wait".

I wonder if there is some common way that this works for modal popups.

I saw this again in 3.0.x-dev, should I open a new issue?

brad.bulger → created an issue.

I've changed my installed copy to dev-3.x so I had to make a couple small changes to the patch to remove code that was already there. It seems to work fine.

Can someone who understands this paragraph expand it with examples, and confirm that it works as described?

The path type will create a symlink to your local copy. To clone your local copy instead, set the type to vcs. If your local copy does not contain a composer.json file, you must use the package type and set the actual path in the dist part (to symlink) or the Git URL in the source part (to clone).

When it says "set the type to vcs", which type is "the type"? Can we point to any documentation for creating a composer.json file for a module?

I have a local copy of a contributed module. I'm trying to use a package repository entry of type "drupal-module" to get composer to install it into web/modules/contrib/ like a normal contrib module. But there doesn't seem to be any way to get it to make a copy, rather than a symlink.

Not for me. Not clearing the cache with drush, at any rate. And the site does not come up at all to try to clear the cache through the UI. Trying to access /core/rebuild.php does not repair the problem. Running /update.php does repair the problem.

Would that be better than trying to segregate that functionality into another module?

brad.bulger → created an issue.

brad.bulger → created an issue.

This is off-topic, but has there been any thought of adding the start and end date values as separate field choices, similar to how for multivalue fields your choices are field_myvalue and field_myvalue:delta?

I spun up a test site on simplytest.me using 10.2 and extended_html_filter 1.0.x, applying the patch https://git.drupalcode.org/project/extended_html_filter/-/merge_requests... (that link should be good for 12 hours, then you will need to recreate the site)

This shows the first issue I described: if you enable the filter and try to save the format, you get an error that <* style> is missing.

Trying to enable Source editing and add <* style> to the allowed tags fails:

The following attribute(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: Global `style` attribute (<* style>).

I can't attach config for this text format because I can't save these changes due to the error.

I don't have the change to the ckeditor5.yml file around anymore, unfortunately. Our development is stuck in 9.5 for the moment, I don't think it's worth testing in this too much. I'm attaching config files for what it's worth.

Ah, I had missed that change.

Yes, it seems complicated. Possibly the changes in ✨ Match the filter_html <> ckeditor5 integration in Drupal core Needs work address this, or need to do so.

The only AJAX change from the core "Limit allowed HTML" filter that I see is from the default elements allowed without CKEditor5
<a href hreflang> <em> <strong> <cite> <blockquote cite> <code> <ul type> <ol start type='1 A I'> <li> <dl> <dt> <dd> <h2 id='jump-*'> <h3 id> <h4 id> <h5 id> <h6 id>

to the default elements allowed with CKEditor5
<br> <p> <h2> <h3> <h4> <h5> <h6> <strong> <em>

Adding <p> and <br> to the settings for "Extended HTML filter" just moves to another error:

The current CKEditor 5 build requires the following elements and attributes:
<br> <p> <h2> <h3> <h4> <h5> <h6> <strong> <em>
The following elements are not supported:
<a href hreflang> <cite> <blockquote cite> <code> <ul type> <ol start type="1 A I"> <li> <dl> <dt> <dd> <h2 id="jump-*"> <h3 id> <h4 id> <h5 id> <h6 id> <* style lang dir="ltr rtl">

That <* style> is wrong, in that it would allow any value for the style attribute, instead of the whitelisted values in the plugin config.

And in any case, trying to add <* style lang dir="ltr rtl"> to the Allowed HTML tags causes a fatal error in the validation:

Uncaught PHP Exception InvalidArgumentException: ""__zqh6vxfbk3cg__" is not a valid HTML tag name." at /path/to/drupal/web/core/modules/ckeditor5/src/HTMLRestrictions.php line 177,

There is core code expanding wildcards to that arbitrary string value, which is fine for attributes but not so much for elements.

This patch file spells out the changes that worked for me.

brad.bulger → created an issue.

Is comment 17 above saying that at the moment there is no edit capability, only create? That's how it is working for me.

Should this work in CKEditor 5 on Drupal 9.5 using version 2.1.x-dev? I am not seeing a toolbar button. No errors in the log or the browser.

brad.bulger → created an issue.

brad.bulger → created an issue.

I ended up having to change extended_html_filter.ckeditor5.yml to set the <* style> element to match the styles I defined on the Extended HTML config page. Otherwise, it seems it wants that unrestricted style attribute and gives a validation error on the restricted style values from the plugin config.

I wasn't using the issue fork code properly. I've moved to a vanilla Drupal 10 installation, with none of the other CKEditor modules I had installed in the first instance. I create a new text format, that uses CKEditor. The only buttons enabled are the default ones: Headings, bold, and italic. The only filter enabled is Extended HTML Filter. Now the "Allowed HTML tags" field of the Extended HTML filter settings is read-only, like the core Limit HTML filter. The tags enabled are <br> <p> <h2> <h3> <h4> <h5> <h6> <strong> <em>. Trying to save this, I get an error:

The current CKEditor 5 build requires the following elements and attributes:
<br> <p> <h2> <h3> <h4> <h5> <h6> <strong> <em> <* style dir="ltr rtl" lang>
The following elements are missing:
<* style>

The core "Limit allowed HTML" filter is taking other plugins into account in the read-only list of allowed HTML that it generates. The "Extended HTML Filter" is not, at least not for me.

For example, in the Alignment plugin settings, I have all of the available alignments enabled: left, center, right, and justify. In the HTML tags allowed by "Limit allowed HTML", it includes alignment classes on the header tags: <h2 class="text-align-left text-align-center text-align-right text-align-justify"> etc. The tags for "Extended HTML" only include the "id" attribute for the header tags - except for <h2 id='jump-*'> which I don't recognize offhand.

Maybe make these choices configurable in the text format? I would add a suggestion to add a "link text" field or some better name. If the user has selected text in the field being edited, default to that, if not, maybe default to the entity title. Add a configuration option to choose the base path, the alias if exists, or the title as the default. This is very typical capability in text editors.

I really appreciate this, it gets us off the radar of the checkbox checkers.

Is it actually true that you can add a libraries key to a module's info.yml file?

I can confirm that I am able to define custom colors in the CKEditor plugin settings for Font Colors after applying the patch from 🐛 TypeError: array_filter(): Argument #1 ($array) must be of type array RTBC . I had to disable the button, clear the cache, and then enable it again to get it to show up.

I guess the way to add a color to the default palette would be to export, edit, then import the config?

We have this same issue. We can work around it somewhat by creating classes on SPAN in the Style plugin, but it's awkward and people have to use trial and error to pick the right color. I don't know how you could do it dynamically though. We get flagged by security if we have STYLE elements in the page HTML.

should that test on is_array() rather than empty()?

I'm not sure what you all mean when you say the patch is not needed? The basic issue of not being able to insert a media entity image inline still exists in 10.1.x-dev. The image button has an "in line" setting but not Insert Media.

Is this testing relative paths that resolve outside of the Drupal webroot? For example, I've been testing a setup where Drupal is installed in a subdirectory on the server, using $settings['file_public_path'] = '../public';. So a Drupal page would be https://my.server/path/to/drupal/web/node/1 and an uploaded image would be https://my.server/path/to/drupal/public/image01.png. That doesn't work with image styles, so I tried setting file_public_base_url to 'https://my.server/path/to/drupal/public'. That has many of the same symptoms described here.

Is adding the clean URL redirect lines to the public file folder .htaccess file going to be a requirement in any case? In order to do things like process the virtual URLs of image styles, for instance.

file_public_path has to be set to something that is actually inside the docroot. it gets used in file URLs and any leading '..' components are trimmed away, becoming eg 'https://my.website/myFiles/image01.jpg'. which doesn't exist. if the files are in a directory outside the docroot, you can use a symlink to access them. eg make sites/default/files a link to ../../../myFiles.

It's such a small change, and has been like this for so long apparently, I feel like I must be missing something.

brad.bulger → created an issue.

If it is being left to the themes to add the class, but the core code expects it to be there, is that an issue with the core Text module, or the textarea field widget?

And in any case that is irrelevant, it is still a bug somewhere.

No I'm not.

Website is the out of the box default on simplytest.me. The standard website theme is Olivero. The admin theme is Claro.

If you use the website theme to edit content, then you run into this problem.

That is not "using it as an admin theme", it's editing content.

brad.bulger → created an issue.

Still seeing this in the 2.0 beta. If I install domain with drush, the site is inaccessible with this error. I can install the Domain modules through the website admin UI without running into this problem.

I think all of the other links.task.yml files are up to date.

brad.bulger → created an issue.

What does "services.yml is out of date" mean? It's a custom file, how does it get out of date?

This may be related to this core issue where updates to config leave debris in key_value - 💬 How to fix "non-existent config entity name returned by FieldStorageConfigInterface::getBundles()" Needs work

If this might be helpful. The situation is that I deleted and recreated the group type "hq_program_group". There is a field on the user membership relationship, "f_title".

This is what's in config:

> \Drupal::entityTypeManager()->getStorage('field_storage_config')->load('group_relationship.f_title')->getBundles();
= [
    "subgroup-group_membership" => "subgroup-group_membership",
    "mission_subgroup-9376111be951852" => "mission_subgroup-9376111be951852",
    "public_subgroup-group_membership" => "public_subgroup-group_membership",
    "mission_group-group_membership" => "mission_group-group_membership",
    "research_group-group_membership" => "research_group-group_membership",
    "espo_group-group_membership" => "espo_group-group_membership",
    "hq_program_group-group_membership" => "hq_program_group-group_membership",
    "hq_program_group-9376111be951852" => "hq_program_group-9376111be951852",
  ]

Those last two entries are the same, but only one of them works. I don't know why there are two for this one, when for instance the similar case of mission_subgroup doesn't have this problem. So this may be a result of the whole history of this installation, which would make it hard to recreate.

brad.bulger → created an issue.

I am able to create a view based field in the UI, but when I try to import the field definition from config/install/ in a custom module, I get that same TypeError.

brad.bulger → created an issue.

Is this any more or less feasible now in Group 3 and beyond? I didn't see any reference to group_content or group_relationship in any of the "make group revisionable" issues, but there are many of them, I may have missed it.

Not to reopen this, but there are uses for something like this. As an example, we need to be able to track past members of a group. The only way to do that is leave them as members, and then distinguish active and inactive members in permissions. So the insider/outsider member roles aren't used for much. It would be nice to be able to give new members a default role or set of roles that can be removed/changed later.

I guess this should be a contrib module, since it's all external to the main module function.

For future visitors - group_clone will let you clone a group but not a group type.

brad.bulger → created an issue.

I've moved the code to replace 'expire' indexes with 'expire_create' from the fake hook_update_N function into system_requirements(). In the runtime phase (eg when you load the Status report on your site), it will report a warning that the update script needs to be run if it finds at least one 'expire' index on a cache table. In the update or install phases, if it finds an 'expire' index on a cache table, it will drop it and then create an 'expire_create' index if one does not already exist, reporting all of these actions to the user. So running drush updatedb or the update.php page should clean up the indexes even if there are no update hooks needing to be run.

I reset the system module schema number back to before the first hook from this patch that we applied to our site - 7074 - and reran all of the real system module hooks since then, without much incident. YMMV

OK, so if future cache table use (or are supposed to use) the basic cache table schema, then there's no need to redo any of these changes for future updates. Is that right?

It seems like putting the update hook into a new module would be a better way to handle that. There are some alternate caching modules mentioned above, it's over my head to evaluate them though.

I've never been quite clear about the system_update_N function in this patch. It tries to find all existing cache tables and replaces any existing "expire" index with an "expire_created" index. Running in an update function like this, that's a one-time check, right? If new cache tables are created with an "expire" index, this would do nothing about that. I don't know if that would happen. But if a one-time check will suffice, then it seems like instead of interfering with the real update hooks of the system module, that check should move somewhere else.

Is continually bumping up this fake update_N hook in the system module preventing the real hooks from running? Like, d7-cache_garbage_collection-891600-119.patch defined system_update_7086(). Now core Drupal 7.98 has a real system_update_7086() function. But won't my installation think that it has already run?

Closed but never solved, very nice. Maybe it doesn't fit "the definition of bug" (lol) but it sure seems like a problem to people who run into it. Another forever-patch.

reroll for 7.98 - removed a trailing whitespace line from node.test

I see. I'd think comments in the files saying something like what you just said would suffice.

Is there an issue for why the Module list page is constantly reporting an error that it was unable to check update status?

Is the project root .gitignore file meant to replace the webroot example.gitignore? The project root file looks to me like it does everything the webroot file does except ignore /vendor/, which doesn't initially exist in the webroot anyway.

I get your general point, that a complicated structure can be made easier to use in the front end. But neither of the scenarios you describe in #4 seem to have anything to do with the question raised in #3 - being able to create a group role that syncs with multiple Drupal sitewide roles. For example, if I have two Drupal roles, "Manager" and "Editor", and I want them both to have the same set of editing permissions on my group type, does it complicate the permission data / calculations to have a single "Outsider Editor" group role with a scope of Outsider and any of the Drupal roles in ("Manager", "Editor")? Versus creating two group roles, one for each Drupal role. That still leaves you needing to create "Insider Editor", probably, and perhaps also an Individual-scope "Editor" role if needed, but it could still be useful.

brad.bulger → created an issue.

brad.bulger → created an issue.

Adding code to DomainMicrositePathProcessor::processInbound() to replace an empty altered path with '/' fixes it for me.

brad.bulger → created an issue.

If it still works as it did before, where it is a module that has to be enabled separately in addition to ggroup, then whatever is easier to maintain would be the way to go, I think - standalone or component of ggroup.

Being able to optionally have mapping apply to descendants would be nice.

I ended up having to truncate the cache_container table to stop it giving me a fatal error related to the service argument change. Is there a better way to deal with that?

Unless one of the maintainers is saying that this will never be done, closing it as "won't be fixed" after only a year is premature.

I am not sure how much this applies to the Group 3+ way of doing things. Each change in the combination of factors results in a distinct single group role. So maybe it is simpler, in that you can map any source group role to any target group role. Or maybe it has to echo the same scopes as Drupal site roles: one mapping for source group role + member of target group, one for source group role + non-member of target group.