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

hgroup and summary do not accept heading elements #16947

Closed
jwylarsen opened this issue Aug 20, 2024 · 4 comments · Fixed by #16962
Closed

hgroup and summary do not accept heading elements #16947

jwylarsen opened this issue Aug 20, 2024 · 4 comments · Fixed by #16962
Labels
type:bug This issue reports a buggy (incorrect) behavior.

Comments

@jwylarsen
Copy link

📝 Provide detailed reproduction steps (if any)

  1. Enter source editing mode
  2. Create an hgroup element and add a heading element
  3. Create a details element with a child summary element and add a heading element
  4. Toggle back to design mode
  5. Toggle back to source mode

✔️ Expected result

Code is preserved as written.

❌ Actual result

hgroup and summary elements are removed

❓ Possible solution

submitting PR to fix this in schema definitions

📃 Other details

  • Browser: Chrome, Firefox
  • OS: Windows 11, Ubuntu
  • First affected CKEditor version: …
  • Installed CKEditor plugins: …

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@jwylarsen jwylarsen added the type:bug This issue reports a buggy (incorrect) behavior. label Aug 20, 2024
@niegowski
Copy link
Contributor

The problem is in invalid configuration of the GHS hgroup element:

{
model: 'htmlHgroup',
view: 'hgroup',
modelSchema: {
allowChildren: [
'htmlH1',
'htmlH2',
'htmlH3',
'htmlH4',
'htmlH5',
'htmlH6'
],
isBlock: false
}
},

It should be:

{
	model: 'htmlHgroup',
	view: 'hgroup',
	modelSchema: {
		allowIn: '$root',  // <- this is missing
		allowChildren: [
			'paragraph',  // <- this is missing
			'htmlP',  // <- this is missing
			'htmlH1',
			'htmlH2',
			'htmlH3',
			'htmlH4',
			'htmlH5',
			'htmlH6'
		],
		isBlock: false
	}
},

According to MDN it's not a regular container so it does not allow other containers and also it allows only headings and paragraphs inside.

@jwylarsen
Copy link
Author

@niegowski That's great. Do you know how to fix the summary element as well? It should accept EITHER $text OR exactly 1 heading element. A little bit trickier.

@Comandeer
Copy link
Member

@jwylarsen, the specification allows more than one heading in the summary element:

Phrasing content, optionally intermixed with heading content.

It means that any number of heading elements is valid according to the specification, e.g. the following HTML is valid:

<details>
  <summary>
    <em>Test</em>
    <h2>Test</h2>
    <strong>Test</strong>
    <h3>Test</h3>
  </summary>
  <p>Lipsum</p>
</details>

Validator agrees.

@jwylarsen
Copy link
Author

@Comandeer Good to know! I was just going off of what MDN said. Either way, I've submitted a PR to address both hgroup and summary issue here: #16951

illia-stv added a commit that referenced this issue Aug 26, 2024
Fix (html-support): hgroup and summary elements should work with source editing. Closes #16947.
@CKEditorBot CKEditorBot added this to the iteration 77 milestone Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants