silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 91 forks source link

Unstyled screen and CMS-specific text #1696

Closed phptek closed 4 months ago

phptek commented 4 months ago

Module version(s) affected

2.1.18 (In a non-CMS project at 5.1.0 of silverstripe/recipe-core which depends on silverstripe/admin.

Description

Under specific circumstances, I can see an unstyled page with CMS-specific content which shouldn't be present in a framework+admin only project.

"I'm sorry, but you can't access that part of the CMS. If you want to log in as someone else, do so below.

You're logged in as WebCMS."

The text is located in vendor/silverstripe/admin/lang/en.yml and vendor/silverstripe/admin/code/LeftAndMain.php. It mentions "CMS" but silverstripe/admin can be used in non-CMS apps too, so this text should be tweaked accordingly (and I guess all of its translations).

How to reproduce

Possible Solution

Looking in LeftAndMain.php:642 there's this weird thing:

$this->suppressAdminErrorContext = true;

Setting this to false renders the screen much more nicely (though in an unrecognisable style) and note the text itself which doesn't mentions the above content, so that should be injected into whichever template is being used.

Before:

Screenshot_blank_error_2024-02-29

After:

After_2024-02-29 14-26-19

Additional Context

I have checked the "I have reproduced" box, however, I haven't really as it would take a prohibitively long time to install on my Docker setup. I've actually seen this issue many, many times before as far back as v3 if memory serves. Happy to provide further feedback if you need it.

Validations

GuySartorelli commented 4 months ago

I have checked the "I have reproduced" box, however, I haven't really as it would take a prohibitively long time to install on my Docker setup.

Thank you for your honesty.... it sounds like you have a problematic docker setup. I recommend trying DDEV which has a quickstart for Silverstripe CMS.

Under specific circumstances, I can see an unstyled page with CMS-specific content which shouldn't be present in a framework+admin only project.

"I'm sorry, but you can't access that part of the CMS. If you want to log in as someone else, do so below.

You're logged in as WebCMS."

If you were trying to access a route in /admin/* that your user lacks permissions for, then this should be present - it's telling you that you can't access what you were trying to access. Is your problem with the term "CMS"? The term "CMS" does not just mean the silverstripe/cms module, which is really just the "pages" module. "CMS" refers to the administration UI in general in this context.

If your problem is with the styling, it is likely that your user doesn't have any CMS-related permissions. We used to use the admin styling for that message for users who have no CMS-related permissions, but that was raised as a bug. The error therefore displays on the front-end. Since it's on the front-end it's up to your theme to provide appropriate styling.

phptek commented 4 months ago

Thanks for this. I'm more familliar with Lando, but I'll take another look at DDEV at some point. I don't have anything wrong with my Docker setup, it just takes a massive mental leap to move off/away from a well-known project setup (this isn't a client project I'm working FYI but a standalone app - hence the "CMS" thing).

If you were trying to access a route in /admin/* that your user lacks permissions for, then this should be present - it's telling you that you can't access what you were trying to access.

Yes, the message should be there. My beef is a). with it being 100% un-styled and b). using CMS nomenclature.

I think this screen (perhaps others too) need a default style attached. Like I mentioned, if I set $this->suppressAdminErrorContext = true; then with no additional config, that particular error message is styled, so the question becomes: "What is/was the arbiter between one message being styled and another not"? I mean, I know the answer: "it wasn't planned, it's just how it's been for yonks". I get that, so I'm asking if all messages can have a default style attached to them. Using a strict interpretation of your rationale, then the message which is styled, should become un-styled no? ;-)

Is your problem with the term "CMS"? The term "CMS" does not just mean the silverstripe/cms module, which is really just the "pages" module. "CMS" refers to the administration UI in general in this context.

If that's the case, and I've been working with Silverstripe since 2011, then I very much missed the memo on that. IMHO the word "CMS" belongs with the CMS module. Full stop. silverstripe/admin is an intermediary package which can operate in both CMS and non-CMS contexts by design. As such, it should be linquistically neutral as to the context it's to be used in. FWIW, I had no idea this message existed and further suprised it was presented unstyled. Using the same rationale, can you imagine if an edge-case 50x message generated in the ordinary use of silverstripe/cms presented as unstyled? The userland assumption of course is that if the system presents a user with a UI, it should look like the intended designs.

Cheers!

GuySartorelli commented 4 months ago

I think this screen (perhaps others too) need a default style attached. Like I mentioned, if I set $this->suppressAdminErrorContext = true; then with no additional config, that particular error message is styled, so the question becomes: "What is/was the arbiter between one message being styled and another not"? I mean, I know the answer: "it wasn't planned, it's just how it's been for yonks". I get that, so I'm asking if all messages can have a default style attached to them. Using a strict interpretation of your rationale, then the message which is styled, should become un-styled no? ;-)

It's not just styled - it's rendered inside the administration UI context (i.e. the CMS). As per the linked issue, we explicitly don't want users who have no CMS permissions to have anything rendered in the CMS ever.

The difference between the two messages is which context they're rendered in. The one in the CMS context is rendered with CMS styling. The one in a front-end context is rendered with front-end styling. We intentionally do not provide any default styling for the front-end (not even for the WYSIWYG for things like aligning images). That's per design and isn't going to change.

michalkleiner commented 4 months ago

I think I slightly lean towards @phptek's understanding of how one interprets the word 'CMS'. For me a project with just the framework and admin module I wouldn't call the back-end/the admin area a CMS. It's the admin area. Yes, you can manage your models and their data, it doesn't have to be 'content'. Can be settings, options, configuration etc. I also get Guy's view that the cms module basically just adds site tree and pages (simplified for the sake of the discussion), so not much on top of the admin in terms of managing content, it's more about its organisation.

GuySartorelli commented 4 months ago

There's a lot more than just this message if we wanted to change the way we refer to the administration UI - for example LeftAndMain permissions start with CMS_ACCESS. Granted it is in large part because the admin module was ripped out from the cms module, and maybe the change in the way it is referred to could be done in a major release, but I don't think there's a scenario where we change just this message and not overhaul all references to CMS.

phptek commented 4 months ago

@GuySartorelli Thanks for the considered feedback, I appreciate it.

For now I'm happy to close this, but I might open a new, more comprehensive issue in future to cover all of what you summarise with - "Address (or not) once and for all, how much of 'CMS' we retain in silverstripe/admin".