-
Notifications
You must be signed in to change notification settings - Fork 6
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
Remove duplicate "content" id #84
Conversation
@szakitibi thanks for creating this Pull Request and helping to improve Plone! TL;DR: Finish pushing changes, pass all other checks, then paste a comment:
To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically. Happy hacking! |
The same duplicate id is also present in other default Plone controlpanels:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@szakitibi Thanks, this fix looks good, but there are 2 things missing before it is ready to merge:
- Run
tox -e format
to autoformat the file using zpretty - Add an entry for the change log (see https://6.docs.plone.org/contributing/index.html#change-log-entry)
Tibor Szakmany your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request. Learn about the Plone Contributor Agreement: https://plone.org/foundation/contributors-agreement How to add more emails to your GitHub account: https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/adding-an-email-address-to-your-github-account |
@jenkins-plone-org please run jobs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There is a duplicate
<article id="content"
>` introduced with the controlpanel.plone.app.registry/plone/app/registry/browser/templates/records.pt
Lines 20 to 25 in 2ffffe6
The
main_template.pt
already has one - see https://github.com/plone/Products.CMFPlone/blob/6.0.11/Products/CMFPlone/browser/templates/main_template.pt#L85.As a result there is two
<article id="content">
forportal_registry
controlpanel - view source for https://classic.demo.plone.org/portal_registry.The problem can be eliminated by changing the id, or using a simple metal tag to fill the slot.