946677 - Empty p and div tags lead to bad vertical alignment of TOC and content

Status: RESOLVED INVALID

(developer.mozilla.org Graveyard :: Design, defect, P3)

Description

[Jeremie Patonnier :Jeremie]

Attachment: bad-alignement.png

When a page embed a banner at the top of its content, the banner and the TOC are miss aligned (see the screenshot).

This due to a conjunction of factor:

1. Edit a page
2. Add a banner template at the top (for example {{draft}})
3. In the wysiwyg editor check that this template is within a p tag (it's a very often case).
4. Save the page.
5. The banner appear miss align with the TOC.

This is due to the fact that when the banner is generated with the kuma script, it is surrounded with a div tag:

<p><div>…</div></p>

The HTML spec does not allow such element within a p element. Therefor, the HTML sanitizer tool decide to immediately open/close the p tags:

<p></p><div>…</div><p></p>

The issue comes from that p elements have a style that given them a margin, even if they are empty. So the first empty p element add an extra margin that cause the miss alignment of the banner and the TOC.

Would it be possible to remove the unnecessary empty p elements (or at worst possible to bend the stylesheet to avoid that extra margin)?

Comment 1

[Jean-Yves Perrier [:teoli]]

CKEditor adds these <p> around the <div>.

Maybe we should just add a filter on the "bleach" level to remove any <p></p> on a page. (We also get <p></p> at the end of pages because of CKEditor).

Asking Les and Groovecoder for their always useful opinion.

(Not a launch blocker anyway).

Comment 2

[Jeremie Patonnier :Jeremie]

(In reply to Jean-Yves Perrier [:teoli] from comment #1)
> CKEditor adds these <p> around the <div>.

Actually it's not necessarily CK fault. As templates are process server side and CK is working client side, I suspect the empty <p> are due to the Django HTML sanitizer.

In a CK point of view, it's legitimate to surround some raw text with <p>. It's impossible for it to know that a string will turn into a <div>

Let's see what Luke and Les think about it. I also add James as well as he is the Django guru in the place :)

Comment 3

[Luke Crouch [:groovecoder]]

Technically, bleach can't be specific about empty "<p></p>" elements.

We could start using an html5lib_Filter, but I'll let :lorchard make the final call if that's a good approach.

Comment 4

[Les Orchard [:lorchard]]

We could probably throw an html5lib filter on the output stage to filter out empty paragraphs. Definitely not on the editing stage, though.

Comment 5

[John Whitlock [:jwhitlock]]

This page doesn't show this issue anymore, but both the content and the design have changed. Adding empty or nearly empty tags is still an issue (see bug 1426782). I think some filters should update the content from the editing stage.

Comment 6

[Florian Scholz (Open Web Docs)]

bug 1426782 is tracking the follow-up work here.