From b33605ec06b9fc84534e31f58e8d68d80df139de Mon Sep 17 00:00:00 2001 From: Ian Storm Taylor Date: Fri, 9 Nov 2018 16:39:33 -0800 Subject: [PATCH] fix schema normalization bug for child_max_invalid --- packages/slate/src/plugins/schema.js | 39 ++++++------ .../custom/child-min-max-invalid-default.js | 55 ++++++++++++++++ .../child-min-max-invalid-similar-valid.js | 63 +++++++++++++++++++ .../custom/child-min-max-invalid-valid.js | 39 ++++++++++++ 4 files changed, 178 insertions(+), 18 deletions(-) create mode 100644 packages/slate/test/schema/custom/child-min-max-invalid-default.js create mode 100644 packages/slate/test/schema/custom/child-min-max-invalid-similar-valid.js create mode 100644 packages/slate/test/schema/custom/child-min-max-invalid-valid.js diff --git a/packages/slate/src/plugins/schema.js b/packages/slate/src/plugins/schema.js index 48bb1f5f7c..af00f10d4f 100644 --- a/packages/slate/src/plugins/schema.js +++ b/packages/slate/src/plugins/schema.js @@ -394,8 +394,8 @@ function validateNodes(node, rule, rules = []) { return true } - function rewind(pastZero = false) { - if (index > 0 || pastZero) { + function rewind() { + if (index > 0) { index -= 1 count = lastCount } @@ -422,10 +422,10 @@ function validateNodes(node, rule, rules = []) { const error = validateRules(child, def.match) if (error) { + // Since we want to report overflow on last matching child we don't + // immediately check for count > max, but instead do so once we find + // a child that doesn't match. if (max != null && count - 1 > max) { - // Since we want to report overflow on last matching child we don't - // immediately check for count > max, but instead do so once we find - // a child that doesn't match. rewind() return fail('child_max_invalid', { rule, @@ -441,22 +441,21 @@ function validateNodes(node, rule, rules = []) { // If there are more groups after this one then child might actually // be valid. if (nextDef()) { - // We already have all children required for current group, so this - // error can safely be ignored. + // If we've already satisfied the minimum for the current group, + // then we can rewind and proceed to the next group. if (lastCount - 1 >= lastMin) { - rewind(true) + index -= 1 continue } // Otherwise we know that current value is underflowing. There are - // three possible causes: there might just not be enough elements - // for current group, and current child is in fact the first of - // the next group; current group is underflowing, but there is also - // an invalid child before the next group; or current group is not - // underflowing but it appears so because there's an invalid child - // between its members. + // three possible causes for this... + + // 1. There might just not be enough elements for current group, and + // current child is in fact the first of the next group. If so, the + // next def will not report errors, in which case we can rewind and + // report an minimum error. if (validateRules(child, def.match) == null) { - // It's the first case, so we just report an underflow. rewind() return fail('child_min_invalid', { rule, @@ -466,11 +465,15 @@ function validateNodes(node, rule, rules = []) { limit: lastMin, }) } + + // 2. The current group is underflowing, but there is also an + // invalid child before the next group. + // 3. Or the current group is not underflowing but it appears so + // because there's an invalid child between its members. // It's either the second or third case. If it's the second then // we could report an underflow, but presence of an invalid child // is arguably more important, so we report it first. It also lets // us avoid checking for which case exactly is it. - error.rule = rule error.node = node error.child = child @@ -497,9 +500,9 @@ function validateNodes(node, rule, rules = []) { } } + // Since we want to report overflow on last matching child we don't + // immediately check for count > max, but do so after processing all nodes. if (max != null && count > max) { - // Since we want to report overflow on last matching child we don't - // immediately check for count > max, but do so after processing all nodes. return fail('child_max_invalid', { rule, node, diff --git a/packages/slate/test/schema/custom/child-min-max-invalid-default.js b/packages/slate/test/schema/custom/child-min-max-invalid-default.js new file mode 100644 index 0000000000..2e44188d2c --- /dev/null +++ b/packages/slate/test/schema/custom/child-min-max-invalid-default.js @@ -0,0 +1,55 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + title: {}, + quote: { + nodes: [ + { + match: [{ type: 'title' }], + min: 1, + max: 1, + }, + { + match: [{ type: 'paragraph' }], + }, + ], + }, + }, +} + +export const input = ( + + + + + + + + + + + + + + + +) + +export const output = ( + + + + + + + + + + + + +) diff --git a/packages/slate/test/schema/custom/child-min-max-invalid-similar-valid.js b/packages/slate/test/schema/custom/child-min-max-invalid-similar-valid.js new file mode 100644 index 0000000000..6d0d8b4e0e --- /dev/null +++ b/packages/slate/test/schema/custom/child-min-max-invalid-similar-valid.js @@ -0,0 +1,63 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + heading: {}, + quote: { + nodes: [ + { + match: [ + { + type: 'heading', + data: { level: v => v === 1 }, + }, + ], + min: 1, + max: 1, + }, + { + match: [ + { + type: 'heading', + data: { level: v => v === 2 }, + }, + ], + min: 1, + max: 1, + }, + ], + }, + }, +} + +export const input = ( + + + + + + + + + + + + +) + +export const output = ( + + + + + + + + + + + + +) diff --git a/packages/slate/test/schema/custom/child-min-max-invalid-valid.js b/packages/slate/test/schema/custom/child-min-max-invalid-valid.js new file mode 100644 index 0000000000..4893db1cb4 --- /dev/null +++ b/packages/slate/test/schema/custom/child-min-max-invalid-valid.js @@ -0,0 +1,39 @@ +/** @jsx h */ + +import h from '../../helpers/h' + +export const schema = { + blocks: { + paragraph: {}, + title: {}, + quote: { + nodes: [ + { + match: [{ type: 'title' }], + min: 1, + max: 1, + }, + { + match: [{ type: 'paragraph' }], + }, + ], + }, + }, +} + +export const input = ( + + + + + + + + + + + + +) + +export const output = input