🇺🇸United States @nmangold

This issue has been reported upstream at https://github.com/ckeditor/ckeditor5/issues/15453.

This issue blocks 🐛 When you change the code snippet lang from original to something else it breaks Closed: works as designed .

This issue is related to the Drupal core issue 🐛 [upstream] CKEditor 5 accumulates class properties when switch code language in Code Block Active and also as a CkEditor 5 issue https://github.com/ckeditor/ckeditor5/issues/15453.

nmangold → created an issue.

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

Yes. Everything correct. Thanks!

Also, I think there should be a documentation change, or maybe a note in the theme settings, to explicitly tell developers the node links (including Read more) are hidden by default in the Article and Page teaser configs. Then, it is more clear how to show those links if desired, and people will know how to show/hide those links on new content types.

I am suggesting we solve the second requirement. Simply hide the link field by default using the teaser display configurations.

By default disable "Links" on the teaser display (this is not in the current MR)

I suggested the theme setting just to make it easier for people to be aware the links are hidden by Olivero. The theme setting would ultimately control the teaser display configurations. The theme setting is not necessarily needed, and I realize now it can become out-of-sync if someone set the teaser display configuration directly.

@xmacinfo suggests using a theme setting to control the logic in the template, which would also work, but that is still bypassing the field display configurations which still makes them useless.

The problem statement here is:

"As a site builder, when I set the node Links field to be visible via the content type teaser configuration, the node links are not visible."

The node links are not visible, because Olivero forcefully excludes the field from being rendered within the Twig template. Therefore, the content type teaser configuration is not considered.

Since the requirement for Olivero is to hide the links, we can simply hide the links using the content type teaser configurations by default. This allows anyone wanting to show the links to change those configurations. Not create a subtheme and override the template. Since this was not the initial solution, we will need to modify those configurations for existing sites using an update hook, and document the change.

@xmacinfo The template override (subtheme) is only needed currently to solve the issue. I am not suggesting we require a subtheme in the future. That defeats the purpose of this issue. Also, the links are already styled. The existing config just needs to be used.

@nod_ I am not suggesting a different view mode either. The problem here is that Olivero does not render the Links field in the node--teaser.html.twig template. I think you understand that based on your previous message "Don't explicitly exclude links from the node teaser (this is in the current MR)." However, by doing so, it makes the existing Links field display config useless. In order to show those links currently, a subtheme must be created and then the template overridden to actually render the links field. Hopefully, we are on the same page so far.

The code to which you linked is for building the Links field render array. Yes, the "Read more" link is hard-coded during the build phase to exist in that render array only within the teaser view mode. However, there is still a field display config that is (should be) used to show/hide that entire Links field.

I am suggesting that Olivero remove the links via the field display config, e.g. core.entity_view_display.node.page.teaser, after the standard install creates that config. Not via the template, which is the sledge hammer approach, which makes the links field config useless. The theme setting would just be another layer of abstraction that could toggle the visibility of the Links field within any display mode that exists at the time of it being toggled. Olivero would have the Links field hidden by default on all content type display mode configs.

I think I just realized where you are getting hung up. Maybe the theme setting should be named "Hide the node Links field." Since technically, that is the field name. "Read more" is just one of the links in the list within the render array for that field.

I am aware of this problem, but I had not created an issue yet. So, thanks for creating one.

The copy button JS is bundled with this module, because there is a modification which allows the JS to be dynamically imported and added as as a plugin. However, this has led to the CSS and JS becoming out-of-sync as described.

So for the short-term, I may either pin the CSS, or bundle it also. For the long-term, I would like to figure out how to dynamically import the JS without the modification, and then use the latest released version compatible with the version of highlight.js used.

@nod_ Thank you for joining the discussion, understanding the problem, and streamlining solutions.

Changing the default configuration to hide the "Links" field in teaser display appears to be the simplest compromise, and the implementation that I would have expected given the design requirement.

If you want to automate the config changes for existing sites, then we could use an update hook. The result for existing sites wanting to hide the links would be no change (no links). The result for existing sites wanting to show the links would be more involved, as those sites will likely already have a template override. If that template override considers the links field configuration, then the Links field configuration would need to be changed back after the update. If that template override does not consider the configuration, and forces the links to be rendered, then the theme developer can decide whether or not to remove their override and consider using configuration, or not. Therefore, a change record should be included which describes that scenario.

Personally, I feel the initial design decision to hide the links should have already been documented more obviously, such as in the theme README. Therefore, I think the decision to remove the links by default should added to documentation.

Another option to expose this decision more clearly is to have a theme setting which ultimately drives the Link field display configuration, e.g. "Hide the node 'Read more' link in teasers.", which would be enabled by default.

I confirmed MR 7 can be installed in D11, and appears to work as expected.

@mherchel, Removing the Read more links via the render array in the template breaks other functionality provided by Drupal core. The request here is to include a variation of the design with the Read more link rendered. So, we can remove the overridden template, and instead hide the Read more link via configuration on a per site basis as Drupal core intended.

The field configurations for the Links field (Read more links) still exists. Removing the Links field from rendering in the template results in those configuration being useless, and forces the template to be overridden. So, the argument here is that removing the links from rendering in the template was the wrong approach, as it breaks previous functionality provided by Drupal core. Sites that use the Read More links, or the Statistics module, or any other module that integrates with the Links field will not have those links rendered when using the Olivero theme.

So, if the design decision was to remove the Read more links, there should be documentation and potentially a better implementation for the Links field to not render by default, or have the Read more link configurable within the links field, as examples.

"It is not needed", meaning the Read more link is not needed, is not a sufficient explanation for the breaking change for the other functionality provided by the Links field. The ask is to render the links by default, as it was before, until a better implementation has been provided that does not break the functionality provided the core.

Please do not abandon issues. They will have abandonment issues. (ba-dum ching)

Thank you everyone for your opinions, and contributions to this issue. This issue proves how a small change, or oversight, can be very impactful, and shows both the power and flexibility of Drupal.

It appears there was some confusion and disagreements, but everyone is on the same page now. Hopefully, we can get a review from a maintainer/manager.

I personally care a lot about resolving this issue, and hopefully getting it merged. For those reasons, I engage with this issue and contribute. I realize others are passionate also. However, it is important to...

Be patient. Be kind. Work together.

I confirmed the issue, and fixed.

Thank you for reporting this issue.

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

The language showing in CKEditor is functionality provided by CKEditor. That library is attached to the admin theme. Not the frontend theme. This module will not provide such functionality. You could potentially copy that functionality from CKEditor to your frontend theme, or implement Highlight.js Language Display plugin, as potential solutions.

The other issue mentioned, 3414090, is not related to this request. That issue was related to using several body fields on the same page, for example, and the last body field dictating which language code blocks get highlighted on the page.

I am closing this as works as designed.

Added a functional test that runs the same tests from the Node module that checks for the Read more link, but uses the Olivero theme instead of Stark.

Rewrote the issue summary using the issue summary template.

The root issue appears to be in the core PathProcessorFiles, which converts paths like system/files/path/to/private/file.txt to an internal path of system/files?file=path/to/private/file.txt.

Some examples of issues tracking the problem are the following.

• 📌 Simplify private file routing Needs work
• 📌 Use attributes instead of query string in PathProcessorFiles and stop replacing query string in RouteProvider::getRouteCollectionForRequest() Active
• 🐛 Cannot use a / in route parameter Needs work

I am able to reproduce this issue. The patch does solve the unique scenario with private files, but it should be regarded as a temporary solution. The root issue appears to be in the core PathProcessorFiles, which converts paths like system/files/path/to/private/file.txt to an internal path of system/files?file=path/to/private/file.txt.

Some examples of issues tracking the problem are the following.

• 📌 Simplify private file routing Needs work
• 📌 Use attributes instead of query string in PathProcessorFiles and stop replacing query string in RouteProvider::getRouteCollectionForRequest() Active
• 🐛 Cannot use a / in route parameter Needs work

I think it would be helpful to have a test covering this scenario.

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

Although this fixes the exception, it allows for the path to be entered without the leading slash, which results in a fatal error when the user is redirected.

Also, the steps to reproduce as written requires the path saml_login to exist. The issue can be reproduced by simply removing the leading slash from the default value, /user/login path, and submitting the form.

The menu item on the admin configuration page is updated as described.

nmangold → created an issue.

Already created the MR.

Reroll the patch file.

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

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

nmangold → created an issue.

Thanks for the clarification. So, it is essentially a more graceful failure. In that case, I like the idea of the console log by default for now. So, the content creator is not clueless about why the "invalid" code blocks are not functioning as expected. Especially, with the issue regarding non-aliased languages, #3401537.

Aside from this issue, and potentially other open issues, this module works as expected on fresh vanilla Drupal installs for both 10.2.x and 10.3.x, as I just tested.

@andres.torres I assume your issue is a conflict within your environment. I see the incorrect mime type and CORS request errors in your console log. I suggest investigating those. Others are also using this module successfully.

I have not merged this MR yet as there was some discussion about how to improve the solution. The problem is maintaining the list of aliases. I am leaning towards only supporting the CKEditor 5 code block languages by default as that seems reasonable. Hopefully, this will be easier to solve upstream in the future.

I agree that I am not able to reproduce this issue as written, and #3453745 is a better representation of the issue. So, I am closing this issue in favor of the other solution mentioned.

Thank you for your contribution.

Everything was reproduced exactly as described and the solution looks good. I don't love the idea of the catching the error in the console log by default. I would like that more if it were an option that was disabled by default, and could be configured via the module settings page, e.g. debug mode. That said, I will merge this MR, but I might include that configuration at a later date.

Thank you for your contribution.

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

@idiaz.roncero Thank you for working on this. I have been trying to solve the language alias issue since the start.

As you have discovered, the highlightjs code does not allow getting the aliased languages, and it is not a trivial change. Also, I would rather not maintain that list in the filter due to the maintainability problem.

Can you look at https://github.com/highlightjs/highlight.js/issues/3938, which is the upstream issue regarding this problem. The best solution might be a third-party plugin to bridge the gap, but that is currently beyond my knowledge of library.

Since this issue cannot be reproduced using a vanilla Drupal installation, it seems this is specific to your project. I am closing this issue. Feel free to add more details when those are available. Thanks.

Thank you.

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

nmangold → created an issue.

@shinkula, I tested the module against Drupal 10.2.4 specifically using the steps you outlined to reproduce. Also, I am using it on several sites, and I know it does work as expected.

So, I am trying to figure out what is the scenario that does not work correctly for you. Have you tested against a vanilla Drupal install?

Can you debug what is happening in the file at web/modules/contrib/highlightjs_input_filter/src/Plugin/Filter/HighlightJs.php? The libraries are attached dynamically. Also, view web/modules/contrib/highlightjs_input_filter/js/highlightjs_input_filter.js to see how the language and copy button scripts are registered with hljs.

Unfortunately, I will need more information about how to reproduce the problem you are experiencing.

Hi @shinkula, can you verify if changing the HTML code block to XML will add the JS/CSS?

Also, verify if it works for you with another language, such as PHP? Try placing only a PHP block of code in the page content.

I believe this is a duplicate of https://www.drupal.org/project/highlightjs_input_filter/issues/3401537 🐛 Non aliased languages fail to import and prevent Copy plugin from loading Needs review

HTML is a non-aliased language within the Highlight JS library. Use XML instead. I am working to come up with a better solution.

I appreciate your attempt in helping. However, it seems you do not understand the issue. Purging the cache in Cloudflare temporarily fixes the issue. If the issue were a wrong URL, or redirect setting, then purging the Cloudflare cache would not help.

It is intermittent. It is not all images, and it does not always happen. It is media entity images embedded into CKEditor 5.

The cache settings in the .htaccess are the default from Drupal. The browser and proxy cache setting (Cache-control header) setting is 1 year. Changing that value does not fix the problem.

This does not appear to be a direct issue in Drupal, because the images exist on the server, clearing the cache in Drupal does not help, but after purging the URI of the image in Cloudflare the image will appear normally. So, it is likely a Cloudflare setting that should be changed.

All other details from the original post are spot on.

I am also experiencing this exact same issue. So, I would be interested in any potential solutions.

I resolved the issue by ultimately passing an associative array to BubbleableMetadata->mergeAttachments(). So, the multiple drupalSettings attachments could be merged properly.

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

Great explanation, and thanks for your contributions.

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

Added a Drupal "site".

Changed the wording in the introduction.

I was able to reproduce this issue only when the blocks of the same language are non-consecutive. For example, PHP->PHP does not produce the issue, but PHP->PHP->JS->PHP does produce the issue.

The changes in the MR does fix the issue.

Patch #2 resolved this issue for me.

The language, css c plaintext, is not a valid language. Therefore, no Highlight.js language library exists that matches that language. You need to edit the source and remove the incorrect languages, leaving only the one language you want to be styled.

Incorrect languages that remain when switching the language is an issue with CKEditor 5. Not this module. This module only detects the languages, and applies the stylesheet from Highlight.js.

This appears to be a duplicate of https://www.drupal.org/project/quicktabs/issues/3355556 🐛 Not working on Drupal 10 Needs review .

I could not reproduce this issue either.

@Vivek, Loading the last clicked tab is expected behavior created by the feature request in https://www.drupal.org/project/quicktabs/issues/3035749 → . However, I understand your concern, and I think an option should exist to disable that feature as needed. Especially, when using Ajax to load only tab on page load that may be different than the default.

I have created a feature request with a patch at https://www.drupal.org/project/quicktabs/issues/3390978 ✨ Add an option to disable the Tab History feature Needs review .

nmangold → created an issue.

I could not reproduce the issue using a vanilla Drupal install with the Quick Tabs module and submodules enabled. Do you experience the same issue if you switch to the Olivero theme?

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

I cannot reproduce this issue.

You should be able to use the selectors .views_slideshow_controls_text_next a for the next button, and .views_slideshow_controls_text_prev a for the previous button.

Try the highlight.js Input Filter → module for similar functionality on D9+ with several additional features.

I experienced this issue also. The token used was [node:field_content_image:entity], which I assume worked at some point.

After reading https://www.drupal.org/docs/contributed-modules/metatag/frequently-asked... → , I realized the proper token is [node:field_content_image:entity:field_media_image:entity:url].

nmangold → created an issue.

Test added.

This seemed to be related to https://github.com/ckeditor/ckeditor5/issues/11198.

Feature (engine): Added a new `Model#insertObject()` method for inserting elements defined as objects by model schema (see #11198).

Also, I'm guessing the parent element attributes were not being recognized, which was causing the list to be split. Inheriting all from $blockObject when registering the schema did the trick.

I confirm this issue. Also, the issue can be reproduced by simply embedding a media entity in the middle of the list. CKeditor 5 will terminate the list before the media item, and start a new list after the media item.

Initial content

<ol>
    <li>
        Lorem
    </li>
    <li>
        ipsum
    </li>
    <li>
        dolor
    </li>
    <li>
        sit
    </li>
    <li>
        amet
    </li>
</ol>

Content after adding a media entity

<ol>
    <li>
        Lorem
    </li>
    <li>
        ipsum
    </li>
</ol>
<drupal-media data-entity-type="media" data-entity-uuid="5e6a990d-38a7-434d-a708-ecd281c62f79">&nbsp;</drupal-media>
<ol>
    <li>
        dolor
    </li>
    <li>
        sit
    </li>
    <li>
        amet
    </li>
</ol>

Filter config

langcode: en
status: true
dependencies:
  module:
    - editor
    - media
name: 'Basic HTML'
format: basic_html
weight: 0
filters:
  filter_html:
    id: filter_html
    provider: filter
    status: false
    weight: -10
    settings:
      allowed_html: ''
      filter_html_help: false
      filter_html_nofollow: false
  filter_align:
    id: filter_align
    provider: filter
    status: false
    weight: 7
    settings: {  }
  filter_caption:
    id: filter_caption
    provider: filter
    status: false
    weight: 8
    settings: {  }
  editor_file_reference:
    id: editor_file_reference
    provider: editor
    status: false
    weight: 11
    settings: {  }
  media_embed:
    id: media_embed
    provider: media
    status: true
    weight: 100
    settings:
      default_view_mode: default
      allowed_view_modes: {  }
      allowed_media_types:
        image: image
        remote_video: remote_video

Editor config

langcode: en
status: true
dependencies:
  config:
    - filter.format.basic_html
  module:
    - ckeditor5
format: basic_html
editor: ckeditor5
settings:
  toolbar:
    items:
      - bold
      - italic
      - underline
      - '|'
      - link
      - '|'
      - bulletedList
      - numberedList
      - '|'
      - blockQuote
      - drupalMedia
      - insertTable
      - '|'
      - '|'
      - '|'
      - code
      - sourceEditing
  plugins:
    ckeditor5_sourceEditing:
      allowed_tags: {  }
    ckeditor5_list:
      reversed: false
      startIndex: false
    media_media:
      allow_view_mode_override: false
image_upload:
  status: false
  scheme: public
  directory: inline-images
  max_size: ''
  max_dimensions:
    width: null
    height: null

I have added more features to this module, including a module settings form, route, menu item. Also, the module is dynamically loading assets to optimize page load times.

Is this enough changes to consider? If not, please give me an example of the minimum amount of code to consider being approved.

nmangold → created an issue.

nmangold → created an issue.

I ran into this error when upgrading a site to D10 while the Upgrade Status module was still present. I fixed the problem by using the --no-update option when updating Drupal, and to remove the drupal/upgrade_status package. Then, running the update.

composer require 'drupal/core-recommended:^10' 'drupal/core-composer-scaffold:^10' 'drupal/core-project-message:^10' 'drush/drush:^12' --update-with-dependencies --no-update
composer require 'drupal/core-dev:^10' --dev --update-with-dependencies --no-update

composer remove drupal/upgrade_status --no-update

composer update

composer require drupal/upgrade_status

I ran into an issue after upgrading from 2.x to 3.x on a project. The site was throwing an error similar to "Template xxx is not defined..." I finally figured out the project was still using the older 1.x API which was deprecated in 2.x, and removed in 3.x. Hopefully, this helps someone else.

More details can be found at https://www.drupal.org/docs/contributed-modules/components/registering-t... → , and https://www.drupal.org/docs/contributed-modules/components/registering-t... → .