It would be keen if Drupal core were nice enough to alert you if you tried to register a module and theme of the same name... but it doesn't.

If you name your theme and a module the same name you get name space collisions. E.G samename_block(..) will conflict if you have it declared in both module and theme.

It would save a lot of new Drupal developers frustration if Drupal didn't let you register a theme and active module by the same name.

Also noted here http://drupal.org/node/143020

CommentFileSizeAuthor
#89 interdiff_88-89.txt1.3 KBdanflanagan8
#89 371375-89.patch6.93 KBdanflanagan8
#88 interdiff_85-88.txt3.65 KBdanflanagan8
#88 371375-88.patch6.2 KBdanflanagan8
#87 after-patch.png322.5 KBradheymkumar
#86 after-patch-exception.png322.5 KBMadhu kumar
#85 interdiff.txt2.11 KBKapilV
#85 371375-85.patch2.55 KBKapilV
#83 371375-83_name_conflicts.patch2.53 KBAndrew Answer
#67 371375-67_name_conflicts.patch2.53 KBZeiP
#51 chx_hates_name_conflicts.patch6 KBmatt2000
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch chx_hates_name_conflicts.patch. View
#42 371375-42-drupal7-theme-module-conflict-rp.patch6.69 KBRyan Palmer
Passed: 14676 passes, 0 fails, 0 exceptions View
#12 371375-drupal7-theme-module-conflict.patch6.71 KBAnonymous (not verified)
Failed: Failed to run tests. View
#11 module-conflict.jpg29.63 KBAnonymous (not verified)
#11 theme-conflict.jpg47.45 KBAnonymous (not verified)
#4 drupal7-system-theme-module-name-conflict.patch6.67 KBAnonymous (not verified)
Failed: 11308 passes, 0 fails, 2 exceptions View
#3 drupal-6-system-theme-name-conflict.patch7.15 KBAnonymous (not verified)
Failed: Failed to apply patch. View
#2 module-theme-name-conflict-D6.patch7.15 KBAnonymous (not verified)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anonymous’s picture

Assigned: Unassigned »

So we could prevent users from enabling the module if a theme with the same name is currently enabled and vice versa. Same way modules cannot be enabled if dependencies are missing with the tick box grayed out. Could also do a validation check when form is submitted, just in case a user has both module and theme forms open at the same time, which would also give them the option to try and enable a module and a theme of the same name.

Anonymous’s picture

Okay, here's the patch for D6. I'll now work on the D7 one.

Anonymous’s picture

FileSize
7.15 KB
Failed: Failed to apply patch. View
Anonymous’s picture

FileSize
6.67 KB
Failed: 11308 passes, 0 fails, 2 exceptions View

And here is the one for Drupal 7. I hope this gets accepted into core.

Anonymous’s picture

Status: Active » Needs review
Anonymous’s picture

Priority: Minor » Normal
webchick’s picture

Priority: Normal » Minor

This has caused me to lose more hours of my life than I care to admit. So I'm more than fine fixing this, however I'd love for someone like chx or dmitri who have done development on this form to take a look at this patch.

Could we also get a screenshot of what this looks like when there's a conflict?

webchick’s picture

Priority: Minor » Normal

Huh. Must've cross-posted. Sorry!

webchick’s picture

Issue tags: +DrupalWTF

And while we're at it...

Status: Needs review » Needs work

The last submitted patch failed testing.

Anonymous’s picture

FileSize
47.45 KB
29.63 KB

Well, I'm really sorry for the inconvenience. Here's what it looks like on the module and theme forms when there is a name conflict.

Anonymous’s picture

FileSize
6.71 KB
Failed: Failed to run tests. View

Okay I've fixed the patch and this time it passes the module dependencies test.

Anonymous’s picture

Status: Needs work » Needs review
Berdir’s picture

+  $result = db_query("SELECT name FROM {system} WHERE type='module' AND status=1");
+  while ($module = db_fetch_object($result)) {
+    $enabled_modules[] = $module->name;
+  }

Not sure if these get_enabled functions are needed at all, but they should use the new DBNG syntax..

foreach ($result as $module) {

And for these simple loops, you can use fetchAll() instead.
return db_query("SELECT name FROM {system} WHERE type='module' AND status=1")->fetchAll();

But again, I'm not sure if they are needed at all.

See http://drupal.org/node/310072

webchick’s picture

Well, I'm really sorry for the inconvenience.

Wait. Just to be really clear, I meant that the fact that you can't name a module and theme the same has caused me to lose many hours of my life, NOT you and not this patch. :D That was code for "Thanks a lot for looking at this, it's a really confusing and hard to track down problem when it bites you!" :)

Screenshots look awesome. That's exactly how I was hoping this would be fixed.

Anonymous’s picture

The error message seems a little confusing to me it needs to explain how it conflicts.

Example:
A theme with the name Garland already exists. (A theme and a module cannot have the same name).

chx’s picture

This is IMO way way too much in regards of code vs benefit. Why not unique the name in system and in module_rebuild_cache and in _system_theme_data simply catch the exception and put up an error message? This would be about three lines of coded added at each code place... (try, catch, drupal_set_message).

Anonymous’s picture

Assigned: » Unassigned

I think I'll leave this patch for someone else to polish up for Drupal 7 who is more familiar with the database abstraction layer and coding preferences.

chx’s picture

Rohin Knight, I hope you are not offended. I have asked other community members to chime in here because my solution would be a lot less pretty than yours but I do not feel the need for that lot of code. Let's see what others say

zoo33’s picture

Well, this has happened to me too. I guess chx's simple approach would be *good enough* since it would at least stop people like me from doing the wrong thing. A unique DB key is a simple and full proof solution. Coupled with an error message it could even be user friendly, although maybe not as much so as the submitted patch.

cburschka’s picture

Let me see if I got this right:

- add "name" to the "unique keys" in the {system} schema in system.install
- try-catch the insert queries on that table
- warn about duplicate system names in the catch block

Is this all that needs to be done for chx's approach?

chx’s picture

Yes.

cburschka’s picture

While experimenting with this, I have found several caveats that may make this non-trivial:

1.) The upgrade function will need to find duplicates, and follow a certain rule to remove them, because the constraint can otherwise fail to be added to an existing table. The easiest resolution is deleting the theme, since things are less likely to break.
2.) Themes are currently refreshed by deleting the records and reinserting the newly discovered ones, two queries in total. If the insertion fails because a new theme shares the name of a module, then we are left without a single theme in the database. (That is not good.) The catcher will need to switch to the maintenance theme or try to reinstall Garland at least.

sun’s picture

subscribing

Since I was the one who wrote a similar handbook snippet elsewhere...

Status: Needs review » Needs work

The last submitted patch failed testing.

matt2000’s picture

Seems to me the issues identified in #23 violate the 'we can break you code, but not your data' rule. I don't think we should be deleting things from people's databases under any circumstances, unless we're sure we put it there.

Also, since having the duplicate name in the database is only a potential for problems, and not an absolute bug, it's better to err on the side of not deleting things.

Finally, if someone who knew what they were doing really needed to go against the principle, they could manually hack their database values, without having to hack the database structure, to get a around a restriction in the UI.

So, I think the UI restriction is the better approach than the DB key. Surely we can trim down Rohin Knight's code a bit for some middle ground. Given #23, we'll need more than 3 lines of code for the DB key approach anyway.

tic2000’s picture

What would be the problem in allowing to have a theme and a module with the same name? Why for D7 the preferred approach is to not allow people doing that instead of adding "module_" or "theme_" to the value in the database?

matt2000’s picture

It has nothing to do with the database. It's collision in the PHP function namespace.

When Drupal called foo_block, it has no idea if foo_block is in foo.module or themes/foo/template.php .

That's why the only real solution is to change the function naming convention, e.g, foo_define_blocks for modules and foo_render_block for themes.

I think the possibility of name collision is a bug to be fixed. If nothing else, it's a key DX feature. Drupal core should not define theme_foo and foo_hook.

Berdir’s picture

As I said in the other issue, there is no hook_block anymore in D7. But this can also occur if one module provides theme_something and another module provides hook_something, this is not something that is easy to control.

But I'm also not sure what to do with sites that already have that situation. IMHO, we wouldn't delete data, because you have to disable all themes except garland when upgrading to D7 anyway. Whatever we do, we have to inform the user/admin why his theme/module doesn't show up in the list.

matt2000’s picture

Sure, hook_block is just an easy example to illustrate what we should continue to try to avoid.

kwinters’s picture

We have a large number of sites that this would break if we applied the patch (theme name and module name = 'custom').

In our case, this is an intentional split between changes we think should be in a module (or have to be in a module) vs ones that would go in a theme. There are never any hook collisions, because we only ever use one or the other for a given hook.

Can we just make this a warning rather than actually not letting you do it? Or better yet, throw an error only if there is an actual namespace issue. The check only needs to happen on the theme / module list pages.

wretched sinner - saved by grace’s picture

Marked #511874: New "Video Upload" Module's Name conflicts with one of this Module's components as a duplicate of this issue. What was raised there was a namespace collision between a module and another sub-module, rather than module and theme. However the same fix should work for both...

zroger’s picture

Or better yet, throw an error only if there is an actual namespace issue.

These conflicts can cause problems that are not necessarily namespace conflicts, but are much harder to discover. For instance, if I have example module and example theme, when i define a preprocess function in my theme like example_preprocess_node(), this is interpreted by the theme registry as a module preprocess function, since it has no way to determine where this function comes from. Since the theme registry thinks this is a module preprocess function, it gets called in the middle of the stack, rather than as the last function in the stack.

This example won't cause a PHP error like theme_block(), but does make bugs really hard to find. I think that with issues like this, which have the potential to trip people up and tie up support channels, it is better to keep them from happening at all.

kwinters’s picture

That's a good point about the order of execution, etc.

However, I still think this should warn you on the module / theme screen via drupal_set_message but still allow you to enable it. Otherwise, this will be a headache in upgrading from D6 to D7, and I don't see any additional benefit going from "Hey your stuff might not work, we recommend changing it" to "You have to change it if you want to enable it."

Jaza’s picture

AFAIK, we've always more-or-less "supported" allowing a module and a theme to have the same name. At the DB level, we have an index (which is currently not unique, but which possibly should be unique) for the composite key "type, name" (where type is always 'module' or 'theme'). At the PHP level, we have API functions like drupal_get_path($type, $name). And at the file level, we have ./modules and ./themes, ./sites/foo/modules and ./sites/foo/themes, etc.

So, my point of view is that right now, it's ostensibly "allowed" to give a module and a theme the same name (at least, Drupal certainly gives you that impression); and if that causes issues, then that is a bug. In other words, I'm saying: the bug is not that you're allowed to give them the same name, the bug is that giving them the same name can cause side-effects. And anyway, let's face it, being able to give modules and themes the same name is pretty useful in many cases. So let's work on continuing to support it, and on eliminating the side-effects.

The three main causes of namespace collisions are:

  1. hook_foo() implementations in a module and in a template.php (e.g. hook_theme() defined in both)
  2. modulethemename_preprocess_baz() implementations in a module and in a template.php
  3. a theme_bar() implementation in template.php that collides with a function name in a module (e.g. modulethemename_frontpage() being a themable function in template.php, and a menu callback in a module)

All of these collisions can be fixed by introducing additional new prefixing. My vote is to have the extra prefixing in template.php files, since this would require far less work and extra characters than would be involved in adding the prefixes to every function in every module. Just three are needed:

  1. themename_hookname() becomes themename_hook_hookname()
  2. themename_preprocess_themablefunction() becomes themename_template_preprocess_themablefunction()
  3. themename_themablefunction() becomes themename_theme_themablefunction()
kwinters’s picture

Jaza, my preference would still be to warn the user and make it discouraged.

While allowing you to name a module and theme the same name is a feature, it has limited utility (it doesn't really add any new things you can do) and what you are describing as a way to truly support it involves making the D6 -> D7 port process more painful. So, I'd really rather we add in a warning and leave it at that.

Jaza’s picture

@kwinters: agreed, my proposal would mean a much more painful D7 upgrade path (and worse DX for D7 theme developers), and all to support an edge case that few themers will need. In retrospect, it's clearly ridiculous that such a proposal would actually get implemented and committed to core. Anyway, I think it was good to throw it out there, if only just to demonstrate what is required were we to "fully support" modules and themes with the same name.

Agreed that a strong warning about same-named modules and themes is needed, at the minimum.

sun’s picture

FWIW, we could completely avoid such issues if themes would be treated as modules. That would not only solve this issue, but also the underlying issue why one would want to create a module for a theme in the first place. Additionally, we already have a lot of issues in the queue that would be auto-resolved if themes would be treated like modules. Not to mention the sheer amount of new possibilities this would open up for module developers.

webchick’s picture

@sun: Could you explain what you mean by that?

tic2000’s picture

@webchick
In light of this issue it will mean you can't name two modules using the same name :). Other things are for another issue to discuss.

webchick’s picture

No, I know that. ;) I'm wondering what exactly "make themes act like modules" means. Getting rid of the distinction in the system table? Loading them along with module_load_all() or whatever it is? Giving them .install files?

Ryan Palmer’s picture

Component: theme system » system.module
Priority: Normal » Critical
Status: Needs work » Needs review
FileSize
6.69 KB
Passed: 14676 passes, 0 fails, 0 exceptions View

I think redefining the relationship between themes and modules goes beyond the scope of this issue.

IMHO, this patch is critical because namespace collisions are fairly likely given what should be the proper use of custom theme functions in custom and contrib modules. There are about 130 hooks available to the developer in Drupal core, many of which have innocent sounding names that could double as theme function names to a theme/module developer not aware of such namespace restrictions. Eg: hook_field_info as theme_field_info, hook_comment_view as theme_comment_view, etc.

Re-rolled #12 patch to conform with DBTNG. Is this what we want, or do we just want a big nasty drupal_set_message and maybe a warning in the Status report?

---
This patch is contributed by TP1 Internet 360, a Montreal-based Drupal web studio.

matt2000’s picture

Visual review of patch looks good, though I didn't actually apply it. But testbot did. :-)

How do we deal with this in update.php if a site already has a matching theme & module, but no obvious issues (yet) ?

As kwinters (#31) and I and others have seen, this is not uncommon. Some commercial themes even ship with support modules named the same.

I'd also like to hear sun expand on the idea of 'themes are modules', and while it is beyond (but not outside) the scope of this particular issue, and almost certainly too big to implement for D7 at this point, if it will solve many issues at once, it's worth considering.

Ryan Palmer’s picture

Dealing with update.php -- that's an edge case I hadn't considered. What I am equally concerned about is how this would impact enabling a module via drush. Does anybody actually use the Modules page anymore anyway? :)

matt2000’s picture

This is going to be especially critical if #591794: Allow themes to alter forms and page gets in.

matt2000’s picture

On further thought, the upgrade issue is not fatal, because the upgrade instructions inform the user to switch to a core theme and disable contrib modules before proceeding. ( I just don't always do that, at my own peril...) :-)

Ryan Palmer’s picture

Namespace collision is a pretty big issue with Drupal these days, IMHO. With so many contrib modules implementing their own hooks, it's becoming more and more common to have unexplainable errors and/or performance related issues (see: http://acquia.com/node/1011148#comment-15012) related to namespace collision with hook implements.

I think this patch is a small step towards reducing these collisions. Maybe it's too small, but it's a step regardless.

matt2000’s picture

Status: Needs review » Needs work

0. admin.css: There really isn't an existing class we can reuse instead of introducing another? I can't imagine a situation where a designer would want to specifically style a 'conflicting theme' message different than any other such message.

1. The very last hunk has an incorrect message: 'Incompatible with this version of PHP'

2. If I'm reading this correct, system_enabled_themes() and system_enabled_modules should return arrays, not db objects?

3. Can we have the big red X on theme form, like for incompatible versions, instead of just removing the enabled checkbox?

webchick’s picture

Category: feature » task
Priority: Critical » Normal

While it'd be nice to get this fixed, it's been like this since time immemorial and certainly doesn't qualify as critical.

moshe weitzman’s picture

I think we can go with the chx approach. We just need the upgrader (fix_requirements() function) to check if there is a conflict with the new key and ask user to resolve it before proceeding.

matt2000’s picture

Assigned: Unassigned » matt2000
Status: Needs work » Needs review
FileSize
6 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch chx_hates_name_conflicts.patch. View

Here's an attempt at the chx approach.

It took a little extra code because we currently assume that we succeed every time we try to enable a theme. So I had to make an effort at eliminating that assumption.

moshe weitzman’s picture

Is this really enough? How do we gracefully handle failure in system_update_7051()? There will be plenty of sites with dupes here.

matt2000’s picture

Because system_requirements() is checked before every run of update.php, system_update_7051() will never run until the user resolves any conflicts.

retester2010’s picture

Status: Needs review » Needs work
Issue tags: +DX (Developer Experience), +DrupalWTF

The last submitted patch, chx_hates_name_conflicts.patch, failed testing.

scor’s picture

Version: 7.x-dev » 8.x-dev
podarok’s picture

subscribe

askibinski’s picture

Just wondering if this problem will still be present in Drupal 8, with all the namespacing/PSR-0 changes that are commited?

tanc’s picture

Issue summary: View changes

I've just encountered an issue with Drupal 8 where I had a profile (basically a module) and a theme named the same. Everything appeared to work fine except the libraries of the theme refused to load. Some hours of debugging later I realised that in the LibraryDiscoveryParser when it tries to resolve libraries in buildByExtension it decides whether the extension is a module or a theme. In my case because I had a profile named the same as the theme it decided it was a module and subsequently could not find the associated library files. It was a silent failure (as far as I know) so no real way of finding this out without some step debugging.

So this is one area where it is still a problem in Drupal 8.

David_Rothstein’s picture

Marking for potential Drupal 7 backport; see the linked issue for another "interesting" side effect that can occur here.

mgifford’s picture

Assigned: matt2000 » Unassigned

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jlscott’s picture

I have just encountered this error on Drupal 8.3.4. I had a theme and a module with the same name, with the result that the theme libraries failed to load despite showing up as attached to the page (hook_page_attachments). The failure was silent, so it puzzled me for a while.

ZeiP’s picture

Version: 8.3.x-dev » 8.4.x-dev
Status: Needs work » Needs review
FileSize
2.53 KB

I took a stab at creating a new patch for fixing this in Drupal 8. Attached is the patch, which runs the check only when enabling a theme/module. The error isn't pretty, it works the same way that extension name length check does; basically it just fails. Is there need for a nicer UI notice for this?

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ZeiP’s picture

Re-ran the tests against 8.5.x, still applies and all the tests pass. Hopefully someone finds time to review this.

HongPong’s picture

Semi-related: I haven't seen any documentation anywhere about this but I had a problem when there was a number in the machine name of my child theme, preprocess hooks wouldn't fire. see here for more info: #2922599: Preprocess hooks fail if child theme contains numbers in machine name. I would recommend patching core to kick an error or status notification if a theme has a number in its machine name.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

klonos’s picture

I am facing this problem with a site where there is a warning about a `mothership` module missing:

The following module is missing from the file system: <em class="placeholder">mothership</em>. For information about how to fix this, see <a href="https://www.drupal.org/node/2487215">the documentation page</a>. bootstrap.inc:1143

Obviously, Mothership is a theme and not a module (this base theme is actually being used on the site).

I have checked the database, and the module path in the system table seems to be pointing to the correct directory. It is also listed as a type of theme. So is this a core bug, reporting a theme as a missing module, or is this related to the issue we are trying to solve here?

PS: the theme is not under the usual /site/all/themes dir, but rather inside a custom profile. Also it is for some reason nested inside another subdirectory (for some reason the previous agency must have done this). So the path to the .info file is under /profiles/profile_name/themes/mothership/mothership/mothership.info

wturrell’s picture

In my view, this could be broader - perhaps even as far as triggering an 'error' severity item on the Status report.

Currently the error is only thrown when manually uploading a module/theme or supplying a URL.

The situation that brought me here was:

  • creating a theme directory and .yml files by hand (not uncommon), which happened to duplicate a module name
  • enabling rather than installing it
  • then wondering why the CSS library wasn't loading.

Could we at least do a check when choosing 'Set as default' on /admin/appearance too?

blainelang’s picture

I don't think it should ever let you install the 2nd (module or theme) once one of them is installed. This happened to me a couple weeks ago. It was quite a pain to clear the errors that kept occurring even after un-installing the module that had the same name as the custom client theme.

joachim’s picture

Status: Needs review » Needs work

> I don't think it should ever let you install the 2nd (module or theme) once one of them is installed.

+1

Though I think it's doing that at the moment. It's just that:

+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -136,6 +136,12 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
+        // Throw an exception if a theme with the same name is enabled.

+++ b/core/lib/Drupal/Core/Extension/ThemeInstaller.php
@@ -163,6 +163,12 @@ public function install(array $theme_list, $install_dependencies = TRUE) {
+      // Throw an exception if a module with the same name is enabled.

System module's ModulesListForm needs to catch these exceptions and show the user a message.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dasj19’s picture

Just wasted couple of hours trying to find out why my theme would not load any css styles..
And I used xdebug to dig deep into core ant stumbled upon this code: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Asset%21L...

if ($this->moduleHandler
      ->moduleExists($extension)) {
      $extension_type = 'module';
    }
    else {
      $extension_type = 'theme';
    }

So as far as I get.. a module would steal the libraries from a theme if they share the same name.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Andrew Answer’s picture

Rerolled #67 for D9.1.x, tested on D9.1.8. I think that this simple patch should be extended to be more user-friendly solution like #11.

This annoying 12-years bug is a sample of Drupal-hate, because I wasted several hours trying to understand why my CSS in theme is not executed.

Andrew Answer’s picture

I think that at least, we need to update modules/themes documentation to avoid creating new modules/themes with identical names.
From Theming Guide:
It must be unique. Your theme should not have the same short name as any other module, theme, theme engine, or installation profile you will be using on the site.

KapilV’s picture

Status: Needs work » Needs review
FileSize
2.55 KB
2.11 KB

fixed 7 Custom Commands Failed.

Madhu kumar’s picture

FileSize
322.5 KB

Patch #85 applied cleanly and throw an exception if a theme with the same name is enabled .Adding Screenshot for the reference.

RTBC +1

radheymkumar’s picture

FileSize
322.5 KB

Patch #85 applied successfully

danflanagan8’s picture

This one seems well worth getting in to core, but it's not going to get there without tests!

Here's a patch with appropriate test coverage. Unfortunately it fails! There's some problem where if I install and then uninstall a module, the theme installer still things the module is installed. The module installer does not have the same confusion about themes being installed and uninstalled.

I'm still trying to figure out why. In the meantime the test might as well fail up here for all to see.

danflanagan8’s picture

I found a fix for the bug I found during testing. All I did was change a line in the ThemeInstaller so that it's the same as the analogous line in the ModuleInstaller. I wonder if the bug is somehow related to this issue: #3063687: ConfigFactory static cache is not updated when cache tags are invalidated?

Anyway, Here's a patch with the fix. I know that this undoes some dependency injection, but at least it works and it's consistent with the ModuleInstaller, FWIW.

If you look at the interdiff you'll also see I added in a missing us statement for the test I added in #88. The missing use statement did not cause the test to fail because it failed before the test made it to the missing class. So nothing major to worry about there.

The last submitted patch, 88: 371375-88.patch, failed testing. View results

bnjmnm’s picture

Removing credit for #87 which is a renamed copy of the screenshot provided in the prior comment #86