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

Backport WordPress 5.2.1 fixes #15650

Merged
merged 11 commits into from
May 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ cache:

branches:
only:
- master
- wp/5.2

before_install:
- nvm install
Expand Down
1 change: 0 additions & 1 deletion packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -483,5 +483,4 @@ Undocumented declaration.

<!-- END TOKEN(Autogenerated API docs) -->


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen this around locally as well sometimes. Why does this happen?

Copy link
Member

@oandregal oandregal May 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably related #15626 I'm investigating it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#15679 should fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I actually had to add empty lines to the previous npm release (because of the failure we had) to force learna to detect these as changed. This just reverts those unnecessary changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I actually had to add empty lines to the previous npm release (because of the failure we had) to force learna to detect these as changed. This just reverts those unnecessary changes.

Can someone clarify for me: Do these changes matter? Is there anything further which needs to be done in this branch for them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, nothing left to do here.

<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p>
15 changes: 11 additions & 4 deletions packages/block-editor/src/components/rich-text/editable.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ function applyInternetExplorerInputFix( editorNode ) {
}

const IS_PLACEHOLDER_VISIBLE_ATTR_NAME = 'data-is-placeholder-visible';
const CLASS_NAME = 'editor-rich-text__editable block-editor-rich-text__editable';

const oldClassName = 'editor-rich-text__editable';

export const className = 'block-editor-rich-text__editable';

/**
* Whether or not the user agent is Internet Explorer.
Expand Down Expand Up @@ -116,7 +119,11 @@ export default class Editable extends Component {
}

if ( ! isEqual( this.props.className, nextProps.className ) ) {
this.editorNode.className = classnames( nextProps.className, CLASS_NAME );
this.editorNode.className = classnames(
className,
oldClassName,
nextProps.className
);
}

const { removedKeys, updatedKeys } = diffAriaProps( this.props, nextProps );
Expand Down Expand Up @@ -156,7 +163,7 @@ export default class Editable extends Component {
style,
record,
valueToEditableHTML,
className,
className: additionalClassName,
isPlaceholderVisible,
...remainingProps
} = this.props;
Expand All @@ -166,7 +173,7 @@ export default class Editable extends Component {
return createElement( tagName, {
role: 'textbox',
'aria-multiline': true,
className: classnames( className, CLASS_NAME ),
className: classnames( className, oldClassName, additionalClassName ),
contentEditable: true,
[ IS_PLACEHOLDER_VISIBLE_ATTR_NAME ]: isPlaceholderVisible,
ref: this.bindEditorNode,
Expand Down
27 changes: 16 additions & 11 deletions packages/block-editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import Autocomplete from '../autocomplete';
import BlockFormatControls from '../block-format-controls';
import FormatEdit from './format-edit';
import FormatToolbar from './format-toolbar';
import Editable from './editable';
import Editable, { className as editableClassName } from './editable';
import { pickAriaProps } from './aria';
import { getPatterns } from './patterns';
import { withBlockEditContext } from '../block-edit/context';
Expand Down Expand Up @@ -202,7 +202,6 @@ export class RichText extends Component {
range,
multilineTag: this.multilineTag,
multilineWrapperTags: this.multilineWrapperTags,
prepareEditableTree: this.props.prepareEditableTree,
__unstableIsEditableTree: true,
} );
}
Expand Down Expand Up @@ -473,15 +472,18 @@ export class RichText extends Component {
const boundarySelector = '*[data-rich-text-format-boundary]';
const element = this.editableRef.querySelector( boundarySelector );

if ( element ) {
const computedStyle = getComputedStyle( element );
const newColor = computedStyle.color
.replace( ')', ', 0.2)' )
.replace( 'rgb', 'rgba' );

globalStyle.innerHTML =
`*:focus ${ boundarySelector }{background-color: ${ newColor }}`;
if ( ! element ) {
return;
}

const computedStyle = getComputedStyle( element );
const newColor = computedStyle.color
.replace( ')', ', 0.2)' )
.replace( 'rgb', 'rgba' );
const selector = `.${ editableClassName }:focus ${ boundarySelector }`;
const rule = `background-color: ${ newColor }`;

globalStyle.innerHTML = `${ selector } {${ rule }}`;
}

/**
Expand Down Expand Up @@ -731,7 +733,10 @@ export class RichText extends Component {
const { formats, text, start, end } = value;
const { activeFormats = [] } = this.state;
const collapsed = isCollapsed( value );
const isReverse = event.keyCode === LEFT;
// To do: ideally, we should look at visual position instead.
const { direction } = getComputedStyle( this.editableRef );
const reverseKey = direction === 'rtl' ? RIGHT : LEFT;
const isReverse = event.keyCode === reverseKey;

// If the selection is collapsed and at the very start, do nothing if
// navigating backward.
Expand Down
29 changes: 20 additions & 9 deletions packages/block-editor/src/components/writing-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
* Browser constants
*/

const { getSelection } = window;
const { getSelection, getComputedStyle } = window;

/**
* Given an element, returns true if the element is a tabbable text field, or
Expand Down Expand Up @@ -241,6 +241,18 @@ class WritingFlow extends Component {
const hasModifier = isShift || event.ctrlKey || event.altKey || event.metaKey;
const isNavEdge = isVertical ? isVerticalEdge : isHorizontalEdge;

// When presing any key other than up or down, the initial vertical
// position must ALWAYS be reset. The vertical position is saved so it
// can be restored as well as possible on sebsequent vertical arrow key
// presses. It may not always be possible to restore the exact same
// position (such as at an empty line), so it wouldn't be good to
// compute the position right before any vertical arrow key press.
if ( ! isVertical ) {
this.verticalRect = null;
} else if ( ! this.verticalRect ) {
this.verticalRect = computeCaretRect( target );
}

// This logic inside this condition needs to be checked before
// the check for event.nativeEvent.defaultPrevented.
// The logic handles meta+a keypress and this event is default prevented
Expand Down Expand Up @@ -281,11 +293,10 @@ class WritingFlow extends Component {
return;
}

if ( ! isVertical ) {
this.verticalRect = null;
} else if ( ! this.verticalRect ) {
this.verticalRect = computeCaretRect( target );
}
// In the case of RTL scripts, right means previous and left means next,
// which is the exact reverse of LTR.
const { direction } = getComputedStyle( target );
const isReverseDir = direction === 'rtl' ? ( ! isReverse ) : isReverse;

if ( isShift ) {
if (
Expand Down Expand Up @@ -316,9 +327,9 @@ class WritingFlow extends Component {
placeCaretAtVerticalEdge( closestTabbable, isReverse, this.verticalRect );
event.preventDefault();
}
} else if ( isHorizontal && getSelection().isCollapsed && isHorizontalEdge( target, isReverse ) ) {
const closestTabbable = this.getClosestTabbable( target, isReverse );
placeCaretAtHorizontalEdge( closestTabbable, isReverse );
} else if ( isHorizontal && getSelection().isCollapsed && isHorizontalEdge( target, isReverseDir ) ) {
const closestTabbable = this.getClosestTabbable( target, isReverseDir );
placeCaretAtHorizontalEdge( closestTabbable, isReverseDir );
event.preventDefault();
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/block-library/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,4 @@ registerCoreBlocks();

<!-- END TOKEN(Autogenerated API docs) -->


<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p>
1 change: 0 additions & 1 deletion packages/blocks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -845,5 +845,4 @@ wrapped component.

<!-- END TOKEN(Autogenerated API docs) -->


<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p>
21 changes: 21 additions & 0 deletions packages/blocks/src/api/raw-handling/comment-remover.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* WordPress dependencies
*/
import { remove } from '@wordpress/dom';

/**
* Browser dependencies
*/
const { COMMENT_NODE } = window.Node;

/**
* Looks for comments, and removes them.
*
* @param {Node} node The node to be processed.
* @return {void}
*/
export default function( node ) {
if ( node.nodeType === COMMENT_NODE ) {
remove( node );
}
}
4 changes: 3 additions & 1 deletion packages/blocks/src/api/raw-handling/paste-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { getBlockContent } from '../serializer';
import { getBlockAttributes, parseWithGrammar } from '../parser';
import normaliseBlocks from './normalise-blocks';
import specialCommentConverter from './special-comment-converter';
import commentRemover from './comment-remover';
import isInlineContent from './is-inline-content';
import phrasingContentReducer from './phrasing-content-reducer';
import headRemover from './head-remover';
Expand Down Expand Up @@ -44,7 +45,7 @@ const { console } = window;
* @return {string} HTML only containing phrasing content.
*/
function filterInlineHTML( HTML ) {
HTML = deepFilterHTML( HTML, [ googleDocsUIDRemover, phrasingContentReducer ] );
HTML = deepFilterHTML( HTML, [ googleDocsUIDRemover, phrasingContentReducer, commentRemover ] );
HTML = removeInvalidHTML( HTML, getPhrasingContentSchema(), { inline: true } );

// Allows us to ask for this information when we get a report.
Expand Down Expand Up @@ -204,6 +205,7 @@ export function pasteHandler( { HTML = '', plainText = '', mode = 'AUTO', tagNam
imageCorrector,
phrasingContentReducer,
specialCommentConverter,
commentRemover,
figureContentReducer,
blockquoteNormaliser,
];
Expand Down
43 changes: 43 additions & 0 deletions packages/blocks/src/api/raw-handling/test/comment-remover.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Internal dependencies
*/
import commentRemover from '../comment-remover';
import { deepFilterHTML } from '../utils';

describe( 'commentRemover', () => {
it( 'should remove a single comment', () => {
expect( deepFilterHTML(
'<!-- Comment -->',
[ commentRemover ]
) ).toEqual(
''
);
} );
it( 'should remove multiple comments', () => {
expect( deepFilterHTML(
'<!-- First comment --><p>First paragraph.</p><!-- Second comment --><p>Second paragraph.</p><!-- Third comment -->',
[ commentRemover ]
) ).toEqual(
'<p>First paragraph.</p><p>Second paragraph.</p>'
);
} );
it( 'should remove nested comments', () => {
expect( deepFilterHTML(
'<p>Paragraph.<!-- Comment --></p>',
[ commentRemover ]
) ).toEqual(
'<p>Paragraph.</p>'
);
} );
it( 'should remove multi-line comments', () => {
expect( deepFilterHTML(
`<p>First paragraph.</p><!--
Multi-line
comment
--><p>Second paragraph.</p>`,
[ commentRemover ]
) ).toEqual(
'<p>First paragraph.</p><p>Second paragraph.</p>'
);
} );
} );
1 change: 0 additions & 1 deletion packages/dom/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,5 +268,4 @@ Wraps the given node with a new node with the given tag name.

<!-- END TOKEN(Autogenerated API docs) -->


<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p>
8 changes: 6 additions & 2 deletions packages/dom/src/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,20 +143,24 @@ function isEdge( container, isReverse, onlyVertical ) {
return true;
}

// In the case of RTL scripts, the horizontal edge is at the opposite side.
const { direction } = computedStyle;
const isReverseDir = direction === 'rtl' ? ( ! isReverse ) : isReverse;

// To calculate the horizontal position, we insert a test range and see if
// this test range has the same horizontal position. This method proves to
// be better than a DOM-based calculation, because it ignores empty text
// nodes and a trailing line break element. In other words, we need to check
// visual positioning, not DOM positioning.
const x = isReverse ? containerRect.left + 1 : containerRect.right - 1;
const x = isReverseDir ? containerRect.left + 1 : containerRect.right - 1;
const y = isReverse ? containerRect.top + buffer : containerRect.bottom - buffer;
const testRange = hiddenCaretRangeFromPoint( document, x, y, container );

if ( ! testRange ) {
return false;
}

const side = isReverse ? 'left' : 'right';
const side = isReverseDir ? 'left' : 'right';
const testRect = getRectangleFromRange( testRange );

return Math.round( testRect[ side ] ) === Math.round( rangeRect[ side ] );
Expand Down
13 changes: 13 additions & 0 deletions packages/e2e-tests/config/setup-test-framework.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,19 @@ function observeConsoleLogging() {
return;
}

// A bug present in WordPress 5.2 will produce console warnings when
// loading the Dashicons font. These can be safely ignored, as they do
// not otherwise regress on application behavior. This logic should be
// removed once the associated ticket has been closed.
//
// See: https://core.trac.wordpress.org/ticket/47183
if (
text.startsWith( 'Failed to decode downloaded font:' ) ||
text.startsWith( 'OTS parsing error:' )
) {
return;
}

const logFunction = OBSERVED_CONSOLE_MESSAGE_TYPES[ type ];

// As of Puppeteer 1.6.1, `message.text()` wrongly returns an object of
Expand Down
6 changes: 6 additions & 0 deletions packages/e2e-tests/specs/__snapshots__/links.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,9 @@ exports[`Links should contain a label when it should open in a new tab 1`] = `
<p>This is <a href=\\"http://w.org\\" target=\\"_blank\\" rel=\\"noreferrer noopener\\" aria-label=\\"WordPress (opens in a new tab)\\">WordPress</a></p>
<!-- /wp:paragraph -->"
`;

exports[`Links should contain a label when it should open in a new tab 2`] = `
"<!-- wp:paragraph -->
<p>This is <a href=\\"http://wordpress.org\\">WordPress</a></p>
<!-- /wp:paragraph -->"
`;
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ exports[`RichText should apply multiple formats when selection is collapsed 1`]
<!-- /wp:paragraph -->"
`;

exports[`RichText should handle Home and End keys 1`] = `
"<!-- wp:paragraph -->
<p>-<strong>12</strong>+</p>
<!-- /wp:paragraph -->"
`;

exports[`RichText should handle change in tag name gracefully 1`] = `
"<!-- wp:heading {\\"level\\":3} -->
<h3></h3>
Expand Down
Loading