Skip to content
This repository was archived by the owner on Feb 6, 2023. It is now read-only.

Commit 9246cc7

Browse files
yury-sannikovfacebook-github-bot
authored andcommitted
convertFromHTML breaks after converting \n string, issue #1822 (#1823)
Summary: This diff is commandeering a PR based on a previous baseline to rebase it against latest master. This diff discards the original fix which is no longer applicable, having the affected HTML converter been removed in favor of the new implementation by nicolasc. I am still keeping the test case as a guard against regression. **Summary** The core issue is here: https://github.com/facebook/draft-js/blob/master/src/model/encoding/convertFromHTMLToContentBlocks.js#L376 A call to the `let chunk = {...EMPTY_CHUNK};` create a shallow copy of the `EMPTY_CHUNK` object. It means that `chunk.blocks` array (and others) share the same reference to the `EMPTY_CHUNK.blocks`. If code hit this line https://github.com/facebook/draft-js/blob/master/src/model/encoding/convertFromHTMLToContentBlocks.js#L633 `EMPTY_CHUNK` object will end up having `block` array with one element of `{type: 'unstyled', depth: 0}` which caused the aforementioned issue. Next call to the `convertFromHTML` will start with polluted `EMPTY_CHUNK` object. :-( **Test Plan** Open a https://jsfiddle.net/m6z0xn4r/1190/ Put a breakpoint on `convertFromHTML` and see 2 different results while converting headers Another way: Open chrome dev tools and: - execute `convertFromHTML('\n')` - execute `convertFromHTML('<h1>H</h1>')` Observe that second call return a block with unstyled type. Pull Request resolved: #1823 Test Plan: Adds a regression test case, run with `jest --watch` Reviewed By: gmertk Differential Revision: D17476663 Pulled By: claudiopro fbshipit-source-id: 05da4d53114aa0d09de61ce55c27f5166b021138
1 parent b858f43 commit 9246cc7

File tree

1 file changed

+16
-0
lines changed

1 file changed

+16
-0
lines changed

src/model/encoding/__tests__/convertFromHTMLToContentBlocks-test.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,22 @@ test('Should not create empty container blocks around ol and their list items wh
349349
});
350350
});
351351

352+
// Regression test for issue https://github.com/facebook/draft-js/issues/1822
353+
test('Should convert heading block after converting new line string', () => {
354+
// Convert an HTML string containing a newline
355+
// This was previously altering the module's internal state
356+
convertFromHTML('a\n');
357+
// Convert again, and assert this is not affected by the previous conversion
358+
const contentBlocks = convertFromHTML('<h1>heading</h1>')?.contentBlocks;
359+
expect(contentBlocks?.length).toBe(1);
360+
const contentBlock = contentBlocks?.[0];
361+
// #FIXME: Flow does not yet support method or property calls in optional chains.
362+
if (contentBlock != null) {
363+
expect(contentBlock.getType()).toBe('header-one');
364+
expect(contentBlock.getText()).toBe('heading');
365+
}
366+
});
367+
352368
test('Should preserve entities for whitespace-only content', () => {
353369
const html_string = `
354370
<a href="http://www.facebook.com">

0 commit comments

Comments
 (0)