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

duplicate IDs for formfields #1493

Closed
asaage opened this issue Apr 23, 2018 · 10 comments
Closed

duplicate IDs for formfields #1493

asaage opened this issue Apr 23, 2018 · 10 comments
Assignees
Labels
Milestone

Comments

@asaage
Copy link

asaage commented Apr 23, 2018

In #957 there is dicussion about same form multiple times on a page which causes invalid html because of duplicate IDs.

I noticed, that the ce_comment-form and the mod_registration-form
both use id="ctrl_email"
whereas the mod_unsubscribe-form uses something like id="ctrl_email_xy" (xy being the module-ID)

It would be nice if i could use both forms on the same page and still have valid html.

@m-vo
Copy link
Member

m-vo commented Apr 23, 2018

True, but there are the same problems like in the referenced ticket that changing them would imply a BC break.

You can still alter the template. Maybe this is a known limitation, then.

@fritzmg
Copy link
Contributor

fritzmg commented Apr 24, 2018

Not being able to put two completely different forms on the same page (by default) is rather weird though.

@asaage
Copy link
Author

asaage commented Apr 24, 2018

The fields are not inside the ce_comment-/mod_registration Template.
I'm not quite shure but i think adding a unique id somewhere here could work.
I don't see why this should be a BCbreak (except for possible css-related things).

@asaage
Copy link
Author

asaage commented Apr 24, 2018

The same problem exists for id="ctrl_website" if that is activated in the registration-module.

@leofeyer leofeyer added this to the 4.6.0 milestone May 17, 2018
@leofeyer
Copy link
Member

Adding the module ID to the registration form and the parent ID to the comments form fixes only 99% of the cases (which is good enough I guess). If the parent ID of the comments form happens to be the same as the module ID of the registration form, the problem still exists.

@leofeyer
Copy link
Member

Fixed in 9d87aa2 and contao/comments-bundle@ded4b33.

@frontendschlampe
Copy link
Contributor

May there's a possibility to add a special prefix each module, e.g. nXXX for newsletter, cXXX for comments and fXXX for forms? But I think, it should be a problem, because we have only numeric ID's in Contao, isn't it?

@leofeyer
Copy link
Member

A prefix would not solve the problem, either, if there were two comment forms on the same page.

@fritzmg
Copy link
Contributor

fritzmg commented Jun 25, 2018

I agree that this is good enough. Having two comment forms with the same parent would be a weird setup anyway, wouldn't it.

@frontendschlampe
Copy link
Contributor

A prefix would not solve the problem, either, if there were two comment forms on the same page.

I think 2 comment modules will have different ID's because they're 2 different modules. With prefix you can solve the problem, that a form and a comment module will not have the same ID.

But no problem ... it's ok like it is. If I have the case, that I have 2 modules with same ID I can change the ID of one module.

christian-kolb pushed a commit to christian-kolb/contao that referenced this issue Oct 11, 2018
leofeyer added a commit that referenced this issue Mar 6, 2020
Description
-----------

| Q                | A
| -----------------| ---
| Fixed issues     | -
| Docs PR or issue | -

After adding the context prefixes to the JSON-LD keys in #1457, we did not adjust the HTML markup in the `DefaultIndexerTest` class. Unfortunately, the unit tests now fail.

Commits
-------

ae96975e Adjust the JSON-LD data in the default indexer test
3bebb6be Fix JSON LD parsing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants