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

Firefox: RichText: invisible trailing <br> tags #5872

Closed
maddisondesigns opened this issue Mar 29, 2018 · 36 comments
Closed

Firefox: RichText: invisible trailing <br> tags #5872

maddisondesigns opened this issue Mar 29, 2018 · 36 comments
Assignees
Labels
Browser Issues Issues or PRs that are related to browser specific problems [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Bug An existing feature does not function as intended

Comments

@maddisondesigns
Copy link

Issue Overview

I have a page with a number of different Heading blocks. Some of the blocks display fine on the front-end (e.g. <h3>Some text</h3>) and other headings have an extra <br> tag included (e.g. <h3>Some text<br></h3>)

These couple of headings are displaying fine
screenshot_614

This heading has the extra <br> tag
screenshot_613

When you look at the block with the extra tag, in the Visual Editor, it appears fine. If I click in the block and try to remove any extra (invisible) characters from the end, I can't.

screenshot_617

If I switch to the Code Editor, I can see the extra <br> tag which I never put in and I can't remove (using the Visual Editor).

screenshot_618

Steps to Reproduce (for bugs)

  1. Insert multiple headings and update page
  2. View markup on front-end and see if any include extraneous tags

Expected Behavior

Headers shouldn't randomly include <br> tags

Current Behavior

Headers randomly include extra markup that you can't get rid of

Gutenberg 2.5.0
Firefox Quantum 59.0.2 (64-bit)
WP 4.9.4

@ZebulanStanphill
Copy link
Member

ZebulanStanphill commented Mar 29, 2018

Thanks for reporting this... I experienced this issue back in Gutenberg 2.3 while writing a post, but I forgot to create an issue about it.

I did some quick testing just now in Gutenberg 2.5, and it looks like if you press Shift+Enter to add a new line to a Heading block (and not create a new block), and then press Backspace to remove that new line, a <br/> tag is left behind and can be seen when you edit the block as HTML.

EDIT: I also did a quick test in Gutenberg 0.1, and it looks like the above-described behavior happens in that version as well. So this appears to be an issue that has always existed with the Header block.

@maddisondesigns
Copy link
Author

@SuperGeniusZeb I'm getting <br> tags without even pressing Shift+Enter.

@danielbachhuber danielbachhuber added [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks labels May 9, 2018
@danielbachhuber
Copy link
Member

I'm able to reproduce this without pressing Shift+Enter:

image

testingheadings

@danielbachhuber danielbachhuber added this to the WordPress 5.0 milestone May 9, 2018
@danielbachhuber danielbachhuber added the [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable label May 9, 2018
@danielbachhuber
Copy link
Member

My hunch is that this is an artifact of the Rich Text component, as I don't see anything in the heading block that relates to managing <br> tags.

@yuanworks
Copy link

As @danielbachhuber suspects, it's not related to the heading as this also happens on regular paragraph blocks. However, it doesn't happen when creating a Classic block.

@mtias mtias added Needs Testing Needs further testing to be confirmed. [Component] TinyMCE labels Jul 18, 2018
@sarahmonster sarahmonster mentioned this issue Jul 26, 2018
7 tasks
@brandonpayton
Copy link
Member

I am not able to reproduce this on master. Can anyone else on this ticket still reproduce it?

@designsimply
Copy link
Member

Tested using WordPress 4.9.8 and Gutenberg 4.1.1 using Firefox 62.0.3 on macOS 10.13.6 with the steps provided and I don't see any extra <br> tags added when I check the published post on the front end. (screenshot)

  1. Insert multiple headings and update page
  2. View markup on front-end and see if any include extraneous tags

Closing since the last two reports on this thread note that the problem cannot be reproduced in later versions. If you're still having trouble and can think of something we may be missing in our testing steps, please comment and we'll re-open!

@designsimply designsimply removed the Needs Testing Needs further testing to be confirmed. label Oct 25, 2018
@sarahmonster sarahmonster added the [Block] Heading Affects the Headings Block label Oct 26, 2018
@maxxwv
Copy link

maxxwv commented Nov 2, 2018

I'm seeing this behavior in WP 4.9.8 and Gutenberg 4.1.1. All I do is update the content in a RichText component and the <br> tag is appended to the value.

@danielbachhuber
Copy link
Member

danielbachhuber commented Nov 18, 2018

All I do is update the content in a RichText component and the <br> tag is appended to the value.

I'm still seeing this too (FF 63.0.3).

This is with a custom block using the RichText component:

image

image

@mkaz
Copy link
Member

mkaz commented Nov 19, 2018

Possibly related to #11037

@danielbachhuber is there a way for me to test with your custom block? I can't seem to duplicate this or the issue in #11037 - running latest Gutenberg on 4.9.8

We've seen this issue occur with conflicts with the wpautop priority change and plugins that filter the_content, which should be fixed in this core ticket: https://core.trac.wordpress.org/changeset/43879

@danielbachhuber
Copy link
Member

@mkaz Yes, I'll see if I can put together a custom block that reproduces the issue. This is against latest Gutenberg running 4.9.8

@danielbachhuber
Copy link
Member

@mkaz This snippet reproduces regularly for me:

var registerBlockType = wp.blocks.registerBlockType;
var RichText = wp.editor.RichText;
var __ = wp.i18n.__;


registerBlockType('daniel/test-block', {
	title: __('Daniel Test Block'),
	icon: '',
	category: 'common',
	attributes: {
		title: {
			type: 'string',
			source: 'html',
			selector: 'h3.title'
		}
	},
	supports: {
		className: false
	},
	edit: function edit(props) {
		var attributes = props.attributes,
		    setAttributes = props.setAttributes;
		var title = attributes.title;

		return React.createElement(
			'div',
			{ className: 'medium-6 panel columns' },
			React.createElement(RichText, {
				tagName: 'h3',
				value: title,
				placeholder: __('Enter title…'),
				formattingControls: [],
				onChange: function onChange(value) {
					console.log( value );
					setAttributes({
						title: value
					});
				}
			})
		);
	},
	save: function save(props) {
		var title = props.attributes.title;

		return React.createElement(
			'div',
			{ className: 'medium-6 panel columns' },
			React.createElement(
				'h3',
				{ className: 'title' },
				title
			)
		);
	}
});

Notice the console.log() on the onChange callback. Here's a GIF of the behavior:

extrabrs

Notably, the <br> appears as soon as a space is entered. Here's a second test case that communicates this behavior:

image

@danielbachhuber
Copy link
Member

Also worth noting: the <br> doesn't display in the editor until you've saved and refreshed the Post.

@mkaz
Copy link
Member

mkaz commented Nov 20, 2018

Sorry, clicked the wrong button. I see the same issue in Chrome for me ( v70 on macOS)

@mtias mtias modified the milestones: WordPress 5.0 RC, WordPress 5.0 Nov 20, 2018
@mtias
Copy link
Member

mtias commented Nov 20, 2018

Moving this to 5.0 if it can be addressed.

@ellatrix ellatrix removed [Feature] Blocks Overall functionality of blocks [Block] Heading Affects the Headings Block [External Package] TinyMCE labels Nov 21, 2018
@ellatrix ellatrix self-assigned this Nov 21, 2018
@ellatrix
Copy link
Member

Thanks for the detailed steps @danielbachhuber. I'll work on a fix right now.

@ellatrix
Copy link
Member

Firefox inserts this line break element to ensure the spaces you type at the end of an editable element stay visible. Normally in HTML, spaces, newlines and tabs are used to format HTML. In between words they are reduced to one visible space. Any leading spaces or trailing spaces are simply omitted. Browser work around this problems in different ways. Chrome inserts non breaking spaces, which it (most of the time) removes again when it no longer needs them. This is an equally terrible way of fixing the problem, because when Chrome doesn't remove the non breaking spaces, they mess up the user's content. Firefox fixes the issue with a trailing line break, and doesn't remove it when the line no longer ends with a space, so it gets included in the content as well. We cannot simply remove it because RichText is a controlled component. Any changes to the value flow back into the live DOM. Removing the line break would break Firefox ability to display trailing space characters when typing.

I think the best way to deal with this is to come up with a way to fix this in all browsers the same way and build it into RichText. That's what I'm attempting right now.

@ellatrix
Copy link
Member

Since there should be no visual difference on the front-end either, I'm going to move this issue to 5.0.1.

@danielbachhuber
Copy link
Member

Since there should be no visual difference on the front-end either, I'm going to move this issue to 5.0.1.

@iseulde Just to be clear, there is a visual difference on the frontend: the <br> ends up HTML encoded and ends up in the display of the element.

image

@ellatrix
Copy link
Member

@danielbachhuber Is that with core blocks? I can't reproduce that. Note that if you use RichText in a plugin, you need to save the content with RichText.Content.

@danielbachhuber
Copy link
Member

Is that with core blocks?

@iseulde It's with this code snippet: #5872 (comment)

Note that if you use RichText in a plugin, you need to save the content with RichText.Content.

I don't understand. Can you clarify? My current implementation is this:

React.createElement(RichText, {
	tagName: 'h3',
	value: title,
	placeholder: __('Enter title…'),
	formattingControls: [],
	onChange: function onChange(value) {
		console.log( value );
		setAttributes({
			title: value
		});
	}
})

@ellatrix
Copy link
Member

ellatrix commented Nov 22, 2018

@danielbachhuber What happens if you make some text italic (cmd+i)? Will that also de encoded? I think you need to same the content with RichText.Content in the save function. See https://wordpress.org/gutenberg/handbook/block-api/rich-text-api/#richtext-content.

@danielbachhuber
Copy link
Member

@iseulde Oh, huh. Maybe developer error then. I'll look into this next week.

@ellatrix
Copy link
Member

The issue with the trailing line break is valid, but it's not a blocker. It shouldn't display on the front end if the rich text content is saved correctly. There will just be a trailing line break in the HTML, which isn't that bad.

@ellatrix ellatrix added the Browser Issues Issues or PRs that are related to browser specific problems label Nov 22, 2018
@ellatrix ellatrix changed the title Heading blocks adding random <br> tags Firefox: RichText: invisible trailing <br> tags Nov 22, 2018
@ellatrix
Copy link
Member

#12166 should fix this issue.

@maxxwv
Copy link

maxxwv commented Dec 1, 2018

I am still seeing this as of updating to WP5.0-RC2-43958. Relevant code:

const { registerBlockType } = wp.blocks;
const {
	MediaUpload,
	RichText,
} = wp.editor;
const {
	Button,
	TextControl
} = wp.components;

registerBlockType(
	'aw/floor-plan-in-content', {
		title: __('Floor Plan'),
		icon: 'index-card',
		category: 'common',
		attributes:{
			title: {
				type: 'string',
				source: 'text',
				selector: '.fp_header'
			},
//		...
		},
		edit: props => {
			const {
//			...
				title,
//			...
			} = props;
			return(
				<div className = { props.className }>
					<RichText
						tagName = "h2"
						className = "fp_header widefat"
						placeholder = {__("Floor Plan name")}
						keepPlaceholderOnFocus={ true }
						value = { title }
						onChange= { onChangeTitle }
						multiline = { false }
						formattingControls = { [] }
					/>
//				...
				</div>
			)
		},
		save: props => {
			const {
				attributes: {
					title,
//				...
				}
			} = props;

			return (
				<div className={"floorplan-display" }>
//				...
					<RichText.Content
						tagName = "h2"
						className = "fp_header"
						value = { title }
					/>
//				...
				</div>
			)
		}
	}
);

Console error:

Block validation: Expected token of type 'EndTag' ({ tagName: "h2", type: "EndTag", <prototype> }), saw instead 'Starting' ({ attributes: Array [], selfClosing: false, tagName: "br", type: "StartTag", <prototype> })

Expected:

<div class="wp-block-aw-floor-plan-in-content floorplan-display"> [...] <h2 class="fp_header">Testing again</h2><div> [...] </div></div>

Actual:

<div class="wp-block-aw-floor-plan-in-content floorplan-display"> [...] <h2 class="fp_header">Testing again<br></h2><div> [...] </div></div>

Note the <br> inside my <h1> tag on output. This is breaking validation and causing my block to error on every page (re)load.

@ZebulanStanphill
Copy link
Member

I am still getting <br> tags in the editor when I use the Edit as HTML option in Gutenberg 4.6.1. I'm not getting any on the front-end, though.

@youknowriad
Copy link
Contributor

This PR is not included in a release yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Browser Issues Issues or PRs that are related to browser specific problems [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests