Fixes a bug in StrictHtmlValidationPass: if openStack is non-empty but only contain optional tags, we reported an error directly without checking.
Also refactor the static methods in HtmlTagEntry. Some logic have been repeated in multiple places.
@@ -109,16 +109,28 @@ final class HtmlTagEntry {
* <p>Compared to {@code matchOrError} methods, this method does not report an error when one of
* the deques is empty. However, if both deques are not empty but the top do not match, we should
* report an error.
+ *
+ * <p>Another difference is that we don't try to remove all optional tags from the openStack, if
+ * the closeQueue is empty. It is possible that some of the optional tags are ended in another
+ * control block, and we cannot remove them too eagerly.
*/
- public static void tryMatchOrError(
+ public static boolean tryMatchOrError(
ArrayDeque<HtmlTagEntry> openStack,
ArrayDeque<HtmlTagEntry> closeQueue,
ErrorReporter errorReporter) {
while (!openStack.isEmpty() && !closeQueue.isEmpty()) {
+ // If close tag is not optional tag, but open tag is optional tag, pop the open tag and
+ // continue.
+ if (!closeQueue.peekFirst().isDefinitelyOptional()
+ && openStack.peekFirst().isDefinitelyOptional()) {
+ openStack.pollFirst();
+ continue;
+ }
if (!matchOrError(openStack.pollFirst(), closeQueue.pollFirst(), errorReporter)) {
- return;
+ return false;
}
}
+ return true;
}
/**
@@ -129,17 +141,12 @@ final class HtmlTagEntry {
ArrayDeque<HtmlTagEntry> openStack,
ArrayDeque<HtmlTagEntry> closeQueue,
ErrorReporter errorReporter) {
- while (!openStack.isEmpty() && !closeQueue.isEmpty()) {
- // If close tag is not optional tag, but open tag is optional tag, pop the open tag and
- // continue.
- if (!closeQueue.peekFirst().isDefinitelyOptional()
- && openStack.peekFirst().isDefinitelyOptional()) {
- openStack.pollFirst();
- continue;
- }
- if (!matchOrError(openStack.pollFirst(), closeQueue.pollFirst(), errorReporter)) {
+ if (!tryMatchOrError(openStack, closeQueue, errorReporter)) {
return false;
}
+ // Try to remove any remaining optional tags in the open stack.
+ while (!openStack.isEmpty() && openStack.peekFirst().isDefinitelyOptional()) {
+ openStack.pollFirst();
}
if (!openStack.isEmpty()) {
errorReporter.report(openStack.peek().getSourceLocation(), OPEN_TAG_NOT_CLOSED);
@@ -252,28 +259,7 @@ final class HtmlTagEntry {
}
ArrayDeque<HtmlTagEntry> openStack = openBranch.deque();
ArrayDeque<HtmlTagEntry> closeQueue = closeBranch.deque();
- while (!openStack.isEmpty() && !closeQueue.isEmpty()) {
- // If close tag is not optional tag, but open tag is optional tag, pop the open tag and
- // continue.
- if (!closeQueue.peekFirst().isDefinitelyOptional()
- && openStack.peekFirst().isDefinitelyOptional()) {
- openStack.pollFirst();
- continue;
- }
- // Recursively compare
- if (!matchOrError(openStack.pollFirst(), closeQueue.pollFirst(), errorReporter)) {
- // We have already report an error recursively, so do not need to report again here.
- return false;
- }
- }
- // Check the stack and queue again
- if (openStack.isEmpty() && closeQueue.isEmpty()) {
- continue;
- } else if (openStack.isEmpty()) {
- errorReporter.report(closeQueue.pollFirst().getSourceLocation(), UNEXPECTED_CLOSE_TAG);
- return false;
- } else {
- errorReporter.report(openStack.pollFirst().getSourceLocation(), OPEN_TAG_NOT_CLOSED);
+ if (!matchOrError(openStack, closeQueue, errorReporter)) {
return false;
}
}
