Skip to content

Commit

Permalink
Don't merge sequential unstyled tags
Browse files Browse the repository at this point in the history
This change fixes facebookarchive#738 by ensuring that `convertFromHTMLToContentBlocks` counts unstyled blocks as blocks and by moving the logic to suppress a block's type into that function.

Previously the existence of any tag other than the unstyled tag would cause unstyled blocks not to be parsed as blocks (but, curiously, the aliases of the unstyled tag would be parsed as blocks). This is mean to handle a case like

```
<div><h2>Hello</h2></div>
```

ensuring that the final block is a `header-two` but breaks with examples like

```
<h2>Hello</h2>
<div>world!</div>
<div>Goodbye!</div>
```

which parses as two blocks with the two divs being combined into a single unstyled block.

The core issue is that not counting the unstyled tag when an interesting tag appears anywhere in the document is a global problem to a local issue: the fix is to instead consider each tree under a block individually with unstyled blocks deferring their final type to whatever the next lowest block type is.
  • Loading branch information
colinjeanne committed Jan 9, 2017
1 parent 301d12c commit c4f3611
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 16 deletions.
10 changes: 7 additions & 3 deletions src/model/encoding/convertFromHTMLToContentBlocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ function getListBlockType(
function getBlockMapSupportedTags(
blockRenderMap: DraftBlockRenderMap
): Array<string> {
const unstyledElement = blockRenderMap.get('unstyled').element;
let tags = new Set([]);

blockRenderMap.forEach((draftBlock: DraftBlockRenderConfig) => {
Expand All @@ -170,7 +169,6 @@ function getBlockMapSupportedTags(
});

return tags
.filter((tag) => tag && tag !== unstyledElement)
.toArray()
.sort();
}
Expand Down Expand Up @@ -430,7 +428,13 @@ function genFragment(
}

// Block Tags
if (!inBlock && blockTags.indexOf(nodeName) !== -1) {
if (
(
!inBlock ||
getBlockTypeForTag(inBlock, lastList, blockRenderMap) === 'unstyled'
) &&
blockTags.indexOf(nodeName) !== -1
) {
chunk = getBlockDividerChunk(
getBlockTypeForTag(nodeName, lastList, blockRenderMap),
depth
Expand Down
40 changes: 27 additions & 13 deletions src/model/paste/__tests__/DraftPasteProcessor-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,22 @@ describe('DraftPasteProcessor', function() {
]);
});

/**
* todo: azelenskiy
* Changes to the mocked DOM appear to have broken this.
*
* it('must suppress blocks nested inside other blocks', function() {
* var html = '<p><h2>Some text here</h2> more text here </p>';
* var output = DraftPasteProcessor.processHTML(html, CUSTOM_BLOCK_MAP);
* assertBlockTypes(output, [
* 'unstyled',
* ]);
* });
*/
it('must suppress blocks nested inside other blocks', function() {
var html = '<h2><p>Some text here</p> more text here </h2>';
var {contentBlocks: output} = DraftPasteProcessor.processHTML(html, CUSTOM_BLOCK_MAP);
assertBlockTypes(output, [
'header-two',
]);
});

it('must not suppress blocks nested inside unstyled blocks', function() {
var html = '<div><h2>Some text here</h2> more text here </div>';
var {contentBlocks: output} = DraftPasteProcessor.processHTML(html, CUSTOM_BLOCK_MAP);
assertBlockTypes(output, [
'header-two',
'unstyled',
]);
});

it('must detect two touching blocks', function() {
var html = '<h1>hi</h1> <h2>hi</h2>';
Expand Down Expand Up @@ -194,7 +198,7 @@ describe('DraftPasteProcessor', function() {
]);
});

it('must NOT treat divs as Ps when we pave Ps', function() {
it('must NOT treat divs as Ps when they contain Ps', function() {
var html = '<div><p>hi</p><p>hello</p></div>';
var {contentBlocks: output} = DraftPasteProcessor.processHTML(html, CUSTOM_BLOCK_MAP);
assertBlockTypes(output, [
Expand All @@ -203,6 +207,16 @@ describe('DraftPasteProcessor', function() {
]);
});

it('must treat divs that do not contain Ps as Ps when we have Ps elsewhere', function() {
var html = '<p>hi</p><div>hello</div><div>hola</div>';
var {contentBlocks: output} = DraftPasteProcessor.processHTML(html, CUSTOM_BLOCK_MAP);
assertBlockTypes(output, [
'paragraph',
'unstyled',
'unstyled',
]);
});

it('must replace br tags with soft newlines', function() {
var html = 'hi<br>hello';
var {contentBlocks: output} = DraftPasteProcessor.processHTML(html, CUSTOM_BLOCK_MAP);
Expand Down

0 comments on commit c4f3611

Please sign in to comment.