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

Remove superfluous bogus BR elements #559

Closed

Conversation

sjbufton
Copy link
Contributor

  • IE versions below 11 pad empty elements too much if a <br data-mce-bogus="1"> element is automatically added; the empty element has a height of two lines, not one.
  • Nested empty elements, e.g. <div class="a"><div class="b"></div></div> were transformed into <div class="a"><div class="b"><br data-mce-bogus="1"></div><br data-mce-bogus="1"></div>; the final <br ...> is unnecessary, and again pads the display too much - on all browsers. The extra parameter "bogusElements" to Node.isEmpty() allows elements marked as "data-mce-bogus" to also count as non-empty, and is used when setting the editor content to ensure that only the innermost empty element is padded in this way.

S Bufton added 5 commits October 18, 2015 22:47
…E browsers; the <br> expands the block size unnecessarily on IE.
Currently fails, because nested empty elements are not recognised, resulting in one BR element at each nesting level, which displays as multiple lines.
@spocke
Copy link
Member

spocke commented Oct 21, 2015

Do you have a bug case where this is happening? Steps to reproduce? Haven't seen a lot of reports so a bit curious about this patch.

@sjbufton
Copy link
Contributor Author

It's fairly easy to reproduce: use the 'development' manual test with IE 9 or 10 (11 in the appropriate mode works the same, and makes it more visible in the Developer Tools), and use the JavaScript console to set the content for point 1:

tinymce.editors[0].setContent('<p></p><p>a</p><h1></h1><h1>b</h1><div></div><div>c</div>');

Then use the DOM Explorer (IE11 Developer Tools) to highlight the blocks: the empty elements are significantly taller than the elements with content.

For point two, use IE11 in Edge mode, Firefox, Chrome etc., modify the 'development' page to include extended_valid_elements: 'div[!class]' and set the content as above to <div class="a"><div class="b"></div></div><div class="a"><div class="b">c</div></div>; use the developer tools to compare the heights of the empty and non-empty divs.

@spocke
Copy link
Member

spocke commented Oct 21, 2015

Yes, managed to reproduce it. Need to get a CLA signed though in order for me to pull this in. See on the commits that you use a somewhat anonymous email.

@sjbufton
Copy link
Contributor Author

I thought I had the e-mail address fixed, but it has only set the committer (I rebased my original commits). Sorry. Can you use the committer address for the CLA?

@fyrkant
Copy link

fyrkant commented Oct 11, 2017

I am closing this pull request due to old age and merge conflicts. If you still think this is relevant and something that should be merged, fix the merge conflicts and leave a comment and I can open it up again. Thank you for the contribution!

@fyrkant fyrkant closed this Oct 11, 2017
fyrkant pushed a commit that referenced this pull request Feb 27, 2018
* commit '6e5b84ccb06cd4b26e8c5682d80c25be1a4618e2':
  TINY-1573: fix editor remove orgDisplay regression
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants