Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backend.xml pulls the all scripts again, causing non deterministic JS errors #158

Closed
sneridagh opened this issue Sep 28, 2018 · 11 comments
Closed

Comments

@sneridagh
Copy link
Member

This rule:
<after css:theme-children="head" css:content="head script" />
is present in both rules.xml and in backend.xml, so that causes to be added twice:

screenshot 2018-09-28 at 10 28 52

In case that you are including backend.xml in your rule set.

@sneridagh
Copy link
Member Author

I'll opened plone/bobtemplates.plone#320 in bobtemplates.plone too.

@djay
Copy link
Member

djay commented Oct 1, 2018

@sneridagh backend.xml is to switch between the a themed and "backend" theme versions of the site so problem is how backend.xml is included. It should replace any script and css includes in rules.xml, not be used in conjunction with it. ie the problem is the example here - https://docs.plone.org/adapt-and-extend/theming/barceloneta.html#using-the-barceloneta-theme-only-for-the-backend

@sneridagh
Copy link
Member Author

@djay I always used it the way that it's shown in the docs, and honestly, I think it's the more sensible way to do it (in terms of readability and approachability to newcomers).

Which is your approach, then? branch rules.xml to support both backend calls and non-backend ones? Then why to filter it again in backend.xml? I mean, you already have in backend.xml:

  <!-- Cut down barceloneta without just for backend UI -->
  <rules css:if-not-content="body.viewpermission-view, body.viewpermission-none">

@sneridagh
Copy link
Member Author

sneridagh commented Oct 1, 2018

@djay better thinking about it, I don't even think that we should include a backend.xml here in the p.barceloneta source code, since it's not used by it at all

I would vote for removing it, leave the one in bobtemplates.plone and improve the docs.

/cc @agitator @MrTango @cdw9 @RobZoneNet

@djay
Copy link
Member

djay commented Oct 1, 2018

Sorry you are right. It was filtered inside backend.xml.

So if it's an exclusive-or between backend or themed why would the js get included twice? As long as your theme contains

    <!-- Only theme front end pages -->
    <rules css:if-content="body.frontend#visual-portal-wrapper">

as per the docs.

Getting rid of it would be a bit of nasty backwards in compatibility. It would break all of our themes and plenty of others peoples too I would guess. I never use bobtemplates.
Backend.xml is part of the barceloneta because the point is for custom themes to be able to rely on barceloneta to be there to theme the backend so they don't have to worry about it. That also means it should be tested. Ideally barceloneta should also come with another bundle that just includes whats needed for the backend only.

@sneridagh
Copy link
Member Author

sneridagh commented Oct 1, 2018

@djay good point, you can still pull it from there in your themes, so indeed removing is not an option.

It does get included twice (see the above screenshot), just both conditions are met, then both rules (the one in rules.xml and the one in backend.xml) are applied twice... I always thought that Diazo takes care of deduplication, but it seems not :( (might be because the conditions are different?)

The issue go away as soon as you only include one.

@cdw9
Copy link
Member

cdw9 commented Oct 1, 2018

I've mostly avoided using backend.xml because it does not work consistently, and instead have opted to add styling for Site Setup. The issue I've run into with the latest version is that edit pages do not have the .frontend class, so backend.xml is used. This is likely the intended behavior, but I would prefer to have backend.xml only work within Site Setup.

So I guess my recommendation would be to remove it, and update docs to instruct how users can set it up themselves if they want it.

@djay
Copy link
Member

djay commented Oct 2, 2018

@cdw9 so the bug is the make the body.frontend tag work consistently. The only inconstancy I had noticed was the history view because that is not protected by a permission. The @@edit page is certainly not considered backend so there is no inconsistency there. If you want it styled by your theme then you will need more complex rules.

Perhaps one confusing here is that .frontend is a not the best name. What is going on under the hood is that it is looking up what the permission the current view is protected by. You can be more explicit by using body.viewpermission-none, body.viewpermission-view instead of body.frontend. See https://github.com/plone/plone.app.layout/pull/50/files#diff-6f416e162a73a6671fbf3b692dd3ad7fR191. Then you could include .viewpermission-edit (or some other rule) to include @@edit pages in your themed pages.

Full discussion of this feature is in plone/Products.CMFPlone#465

@sneridagh Looking at the code above they should be opposite from each other. given that .frontend == (.viewpermission-none or .viewpermission-view) so something weird must be going wrong with your rules. This is out of the box barceloneta?

@sneridagh
Copy link
Member Author

@djay it is, in fact is the same that bundles in bobtemplates.plone themes, and as far as I remember it's like this since day one you made the PR back in the day.

@vincentfretin
Copy link
Member

@mauritsvanrees From your comment #159 (comment) can we close this issue?

@mauritsvanrees
Copy link
Member

Yes, the fix is in plonetheme.barceloneta 2.1.7, included in 5.2.2. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants