Settings for non-existent theme

Same happens in 7.x-dev. Attached is an attempt to improve the situation.

Right now, system.module sets up an URL for the gobal settings (build/themes/settings) and one URL for each of the installed themes (build/themes/settings/<key>). Both use the same handler function, and in the case of the theme-specific settings the key is passed as a second argument.

In case the theme does not exist, none of the registered specific URLs are used, and the system falls back to the global settings URL (although still passing the invalid key).

Patch attached just forces a blank key to be passed to the handler in case the theme does not exist. Upshot is no more ugly error messages, drawback is that the user will get the global settings page instead of an error message. Since a) the user is hacking away at the URLs anyway, and b) this seems to be more in line with the set up of the menu links, I think this is an acceptable.

If the above solution is not acceptable, maybe we should return a "page not found"?

re-test.

If you visit admin/buuild or admin/settings/ddsdjf, you will also see the parent page. This approach is okay.

Not sure about the '' string though. NULL seems more appropriate...

Edit: Oops, system_theme_settings has a signature that specifies ''.

In that case, the argument should match the signature. Fine.

I'm wondering if it is necessary to install a URL handler/router for each of the installed themes. In other areas of Drupal we would install one generic handler that handles all themes, and then have a drupal_not_found() when an invalid parameter is passed in. It feels like we can potentially do a better clean-up, but I have not investigated that deeply. I think it is worth investigating though.

Yeah, I agree with that.

It looks like taxonomy_term_page() might serve as a model for this. Wow, I never thought I'd say anything in taxonomy module could serve as a model for anything. ;)

Still a valid issue after the release of 7.0. Moving to 8.x.

Setting back to 7.x-dev: bugs are always fixed in HEAD and then backported. As long as the D8 branch hasn't opened, bugs get fixed in 7.x-dev...

Assigning to self to take a look at later.

Any news on this?

How about this.

Looks good actually....

Looks like this would be easy to test too...

The issue itself was fixed somewhere else, but as far as I could tell it never got tests. Patch attached adds a couple. Anyone know if there's an existing active issue to address the presence of help text on a 404 page?

Needs a reroll.

I wasn't able to find an issue for the help text on the 404 page so I created #2073121: Help text from hook_help() is displayed on 404/403 pages.

Patch re-rolled.

#1: 323926-theme-bug.patch queued for re-testing.

This did not throw an error in the past, because all themes were loaded at all times.

With the fix of the parent issue, only test coverage is needed for D8.

Tagging for backport so we don't forget about it.

Tests look pretty good to me and the patch still applies.

Not convinced this is backportable. I thought you could have disabled themes but still set them to active programmatically in 7.x

Committed/pushed to 8.x, moving back to D7 in case I got that wrong...

I think for D7 the bugfix might just be to make sure the theme exists period rather than caring about enabled/disabled.