🇬🇧United Kingdom @nicholasthompson

Interesting - on your screenshot it looks consistent with some other sites with Gin / Media module... and inspecting the markup I see lots of Claro references... Specifically the one providing the wrapper spacing is `fieldset_wrapper`... but on our non-working site, it is `fieldset-wrapper` and there are no claro references.

I shall poke around.

nicholasThompson → created an issue.

nicholasThompson → created an issue.

Same as Bushra - that patch does not apply against 3.0.2 and I'm not sure what base release this is against (there is no 3.x-dev branch in composer and I hope the 3.0.2 tag isn't against the 2.x-dev branch 😉)

In any case the patch looks fine to merge, to me. Would be good to get a D10 release out.

nicholasThompson → created an issue.

Even better than a patch, a merge request - that is so much easier than making patches!

nicholasThompson → created an issue.

@Ruslan - are you applying it to the tagged release or the dev branch?

as far as I'm aware, patches and MR's should be based against the dev branch, not the most recent tag.

I've fixed the merge conflict on the merge request, however I have a thought... This isn't really an "alter"... its more of a "post_save" hook. If this was an alter, it should really run on the `$values` before line 193.

I have a case where I need to inject additional config so that when the user is created, some behavior can be altered (eg exporting to a 3rd party). This is something that needs to be set on the user object before its sent to be saved.

nicholasThompson → made their first commit to this issue’s fork.

Fixing code formatting

nicholasThompson → created an issue.

Agreed - will need to test, but as an "eyeball test" this looks good.

Thanks @chaitanyadessai

This, so far, seems more reliable for me. Fixed the issue where it was returning the wrong content for the domain.

Also fixes the "no result" cache, which brings out homepage down to 2569 queries (from the original ~5,700).. Query time is down to 400ms and page render to 1.3s (on ddev locally as admin - which is doing a lot of other lookups for admin paths, etc).

I guess we could do a separate module which adds the `/admin` path to the whitelist (I'm aware there is an issue open to rename this, I'm using it for consistency with the current codebase) - this might help further, but would mean you couldn't alias any admin URLs (which is probably a rare thing anyway).

Hmmm - the noAlias stuff isn't right and this has also included some language selection fixes we were using in https://www.drupal.org/project/domain_path/issues/3245281#comment-15102240 🐛 Wrong Language is chosen and no node visibe under certian conditions Needs work but no longer seem to behave the same.

This defaults it to "on".

nicholasThompson → created an issue.

Revised patch - this one includes the index.

Just looked at the query itself... its really odd... why does it return the same ID field twice in two different field names... and why does it join the domain_path table to itself by ID?

I've also just hugely helped the DB by adding a new index on alias, domain_id and language. This didn't help the NUMBER of queries, but it has hugely helped the time; page is down to 1.5s now and DB query time from 2.5s to 0.5s.

Looked onto per-page caching (which is the next massive win, really) and its non-trivial to add (needs new writeCache and key generation + an event listener to write the cache on page close).

Will re-roll the patch in a bit...

Very much work in progress....

I've implemented the static caching for lookup and noPath (like the AliasManager) does...

This brought the homepage down from 5788 queries to 3871 (and knocked 2 seconds off)... But thats still about 3,500 queries too many ;)

The "extend" modules page was 18k queries and thats down to 11k now, too... but that take 45 seconds to load.

(all the above is on a local ddev instance).

Having the same issue in 1.3 too...

@mattjones86 did you make any headway on this?

For reference, this is my change - however I'm almost certain this is not the right fix for everyone.

I have ended up debugging this in #3362515 and in my case, I have a non-multilingual site.

The content (for whatever reason) is in a mix of "en" and "und"... however due to the NULL check, this line will always try to resolve to what I assume is the current language for the content?

$langcode = $langcode ?: $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_CONTENT)->getId();

For some reason I've yet to work out, the node in question is set to "und" (as is the domain source and domain path)... however this causes NULL to change to "en". I'm guessing this is because Drupal defaults the site language to "en" rather than "und"?

This causes the domain_path lookup to fail and it falls back to something else making the page access denied.

This is the same behaviour in 8.x-1.2

The patch from @JeremyFrench does not help in my case... in fact, for me, the best fix is to simply remove the line altogether... I assume this is a poor choice for other sites? Although does this mean other sites should fall back to using the alias for the language of the content, rather than the language of the site itself?

Closing as this seems to be a dupe of #3245281

Looks like this is strongly related to https://www.drupal.org/project/domain_path/issues/3245281 🐛 Wrong Language is chosen and no node visibe under certian conditions Needs work (ironically raised by my colleague!)

Hmm interesting... I have been debugging... looks like `getPathByAlias` resolved `$langcode` to `en` on page load, but it is storing the domain_alias as `und`....

nicholasThompson → created an issue.

Also getting this with Media module...

Error: Call to undefined method Drupal\media_library\Form\FileUploadForm::getEntity() in _image_field_caption_widget_process() (line 49 of /code/web/modules/contrib/image_field_caption/image_field_caption.module)
#0 [internal function]: _image_field_caption_widget_process(Array, Object(Drupal\Core\Form\FormState), Array)

Another handy way of using entity_generate, granted not paragraphs here, but might help someone (or Future-Me)

  _imageLookup:
    plugin: migration_lookup
    migration: migrate_file_managed
    source: field_image/0/fid

  field_media_image:
    plugin: entity_generate
    access_check: false
    entity_type: media
    source: '@_imageLookup'
    value_key: name
    ignore_case: true
    default_values:
      bundle: 'image'
    values:
      name: '@_imageLookup/filename'
      field_media_image: '@_imageLookup'

This creates an entity:image during a node migrate.

Also, the original Merge Request provides a Diff URL too:
https://git.drupalcode.org/project/dfp/-/merge_requests/4.diff

This patch is used on one of our websites at the moment and seems to work fine.

What is needed to get this over the line / merged?

Patrick - you might need a . (dot) before text-muted otherwise it's only going to apply to <text-muted> elements.

.text-muted|Faded secondary text

I've made a little module like this:

function media_editor_widget_styles_editor_js_settings_alter(array &$settings) {
  foreach ($settings['editor']['formats'] as &$format) {

    $format['editorSettings']['stylesSet'][] = [
      'name' => 'Float Right',
      'type' => 'widget',
      'widget' => 'drupalmedia',
      'attributes' => [
        'class' => 'float-right',
      ],
    ];
  }
}

And I get a style option in the dropdown, however it doesn't seem to apply to the element...

I think I might need to add something to the widget config? I've read through media/js/plugins/drupalmedia/plugin.js but I'm not familiar with CKEditor plugins...

I can see `allowedContent` contains an empty classes hash...