Skip to content

Commit

Permalink
fix: Cleaned up whitespace processing and added whitespace tests (#1267)
Browse files Browse the repository at this point in the history
  • Loading branch information
Sub6Resources authored May 16, 2023
1 parent fe896de commit cc00406
Show file tree
Hide file tree
Showing 6 changed files with 347 additions and 25 deletions.
13 changes: 8 additions & 5 deletions lib/src/builtins/styled_element_builtin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -429,11 +429,12 @@ class StyledElementBuiltIn extends HtmlExtension {
.entries
.expandIndexed((i, child) => [
child.value,
if (i != context.styledElement!.children.length - 1 &&
if (context.parser.shrinkWrap &&
i != context.styledElement!.children.length - 1 &&
child.key.style.display == Display.block &&
child.key.element?.localName != "html" &&
child.key.element?.localName != "body")
const TextSpan(text: "\n"),
const TextSpan(text: "\n", style: TextStyle(fontSize: 0)),
])
.toList(),
),
Expand All @@ -444,14 +445,16 @@ class StyledElementBuiltIn extends HtmlExtension {
style: context.styledElement!.style.generateTextStyle(),
children: buildChildren()
.entries
.expand((child) => [
.expandIndexed((index, child) => [
child.value,
if (child.key.style.display == Display.block &&
if (context.parser.shrinkWrap &&
child.key.style.display == Display.block &&
index != context.styledElement!.children.length - 1 &&
child.key.element?.parent?.localName != "th" &&
child.key.element?.parent?.localName != "td" &&
child.key.element?.localName != "html" &&
child.key.element?.localName != "body")
const TextSpan(text: "\n"),
const TextSpan(text: "\n", style: TextStyle(fontSize: 0)),
])
.toList(),
);
Expand Down
11 changes: 8 additions & 3 deletions lib/src/builtins/text_builtin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@ class TextBuiltIn extends HtmlExtension {
StyledElement prepare(
ExtensionContext context, List<StyledElement> children) {
if (context.elementName == "br") {
return TextContentElement(
text: '\n',
return LinebreakContentElement(
style: Style(whiteSpace: WhiteSpace.pre),
element: context.node as dom.Element?,
node: context.node,
);
}
Expand All @@ -45,6 +43,13 @@ class TextBuiltIn extends HtmlExtension {
@override
InlineSpan build(ExtensionContext context,
Map<StyledElement, InlineSpan> Function() buildChildren) {
if (context.styledElement is LinebreakContentElement) {
return TextSpan(
text: '\n',
style: context.styledElement!.style.generateTextStyle(),
);
}

final element = context.styledElement! as TextContentElement;
return TextSpan(
style: element.style.generateTextStyle(),
Expand Down
116 changes: 100 additions & 16 deletions lib/src/processing/whitespace.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,30 @@ import 'package:html/dom.dart' as html;
class WhitespaceProcessing {
/// [processWhitespace] handles the removal of unnecessary whitespace from
/// a StyledElement tree.
///
/// The criteria for determining which whitespace is replaceable is outlined
/// at https://www.w3.org/TR/css-text-3/
/// and summarized at https://medium.com/@patrickbrosset/when-does-white-space-matter-in-html-b90e8a7cdd33
static StyledElement processWhitespace(StyledElement tree) {
tree = _processInternalWhitespace(tree);
tree = _processInlineWhitespace(tree);
tree = _processBlockWhitespace(tree);
tree = _removeEmptyElements(tree);
return tree;
}

/// [_processInternalWhitespace] removes unnecessary whitespace from the StyledElement tree.
///
/// The criteria for determining which whitespace is replaceable is outlined
/// at https://www.w3.org/TR/css-text-3/
/// and summarized at https://medium.com/@patrickbrosset/when-does-white-space-matter-in-html-b90e8a7cdd33
static StyledElement _processInternalWhitespace(StyledElement tree) {
if ((tree.style.whiteSpace ?? WhiteSpace.normal) == WhiteSpace.pre) {
// Preserve this whitespace
} else if (tree is TextContentElement) {
if (tree.style.whiteSpace == WhiteSpace.pre) {
return tree;
}

if (tree is TextContentElement) {
tree.text = _removeUnnecessaryWhitespace(tree.text!);
} else {
tree.children.forEach(_processInternalWhitespace);
}

return tree;
}

Expand All @@ -36,13 +40,95 @@ class WhitespaceProcessing {
return _processInlineWhitespaceRecursive(tree, Context(false));
}

/// [_processBlockWhitespace] removes unnecessary whitespace from block
/// rendering contexts. Specifically, a space at the beginning and end of
/// each inline rendering context should be removed.
static StyledElement _processBlockWhitespace(StyledElement tree) {
if (tree.style.whiteSpace == WhiteSpace.pre) {
return tree;
}

bool isBlockContext = false;
for (final child in tree.children) {
if (child.style.display == Display.block || child.name == "br") {
isBlockContext = true;
}

_processBlockWhitespace(child);
}

if (isBlockContext) {
for (int i = 0; i < tree.children.length; i++) {
final lastChild = i != 0 ? tree.children[i - 1] : null;
final child = tree.children[i];
final nextChild =
(i + 1) != tree.children.length ? tree.children[i + 1] : null;

if (child.style.whiteSpace == WhiteSpace.pre) {
continue;
}

if (child.style.display == Display.block) {
_removeLeadingSpace(child);
_removeTrailingSpace(child);
}

if (lastChild?.style.display == Display.block ||
lastChild?.name == "br") {
_removeLeadingSpace(child);
}

if (nextChild?.style.display == Display.block ||
nextChild?.name == "br") {
_removeTrailingSpace(child);
}
}
}

return tree;
}

/// [_removeLeadingSpace] removes any leading space
/// from the text of the tree at this level, no matter how deep in the tree
/// it may be.
static void _removeLeadingSpace(StyledElement element) {
if (element.style.whiteSpace == WhiteSpace.pre) {
return;
}

if (element is TextContentElement) {
element.text = element.text?.trimLeft();
} else if (element.children.isNotEmpty) {
_removeLeadingSpace(element.children.first);
}
}

/// [_removeTrailingSpace] removes any leading space
/// from the text of the tree at this level, no matter how deep in the tree
/// it may be.
static void _removeTrailingSpace(StyledElement element) {
if (element.style.whiteSpace == WhiteSpace.pre) {
return;
}

if (element is TextContentElement) {
element.text = element.text?.trimRight();
} else if (element.children.isNotEmpty) {
_removeTrailingSpace(element.children.last);
}
}

/// [_processInlineWhitespaceRecursive] analyzes the whitespace between and among different
/// inline elements, and replaces any instance of two or more spaces with a single space, according
/// to the w3's HTML whitespace processing specification linked to above.
static StyledElement _processInlineWhitespaceRecursive(
StyledElement tree,
Context<bool> keepLeadingSpace,
) {
if (tree.style.whiteSpace == WhiteSpace.pre) {
return tree;
}

if (tree is TextContentElement) {
/// initialize indices to negative numbers to make conditionals a little easier
int textIndex = -1;
Expand All @@ -62,9 +148,9 @@ class WhitespaceProcessing {
final parentNodes = tree.element?.parent?.nodes;

/// find the index of the tree itself in the parent nodes
if ((parentNodes?.length ?? 0) >= 1) {
if (parentNodes?.isNotEmpty ?? false) {
elementIndex =
parentNodes?.indexWhere((element) => element == tree.element) ?? -1;
parentNodes!.indexWhere((element) => element == tree.element);
}

/// if the tree is any node except the last node in the node list and the
Expand Down Expand Up @@ -117,9 +203,7 @@ class WhitespaceProcessing {
/// update the [Context] to signify to that next text node whether it should
/// keep its whitespace. This is based on whether the current text ends with a
/// whitespace.
if (textIndex ==
((tree.element?.nodes.length ?? 0) -
1) && //TODO is this the proper ??
if (textIndex == (tree.node.nodes.length - 1) &&
tree.element?.localName != "br" &&
parentAfterText.startsWith(' ')) {
keepLeadingSpace.data = !tree.text!.endsWith(' ');
Expand All @@ -142,11 +226,11 @@ class WhitespaceProcessing {
/// (4) Replace any instances of two or more spaces with a single space.
static String _removeUnnecessaryWhitespace(String text) {
return text
.replaceAll(RegExp("\\ *(?=\n)"), "\n")
.replaceAll(RegExp("(?:\n)\\ *"), "\n")
.replaceAll(RegExp(r" *(?=\n)"), "")
.replaceAll(RegExp(r"(?<=\n) *"), "")
.replaceAll("\n", " ")
.replaceAll("\t", " ")
.replaceAll(RegExp(" {2,}"), " ");
.replaceAll(RegExp(r" {2,}"), " ");
}

/// [_removeEmptyElements] recursively removes empty elements.
Expand All @@ -155,7 +239,7 @@ class WhitespaceProcessing {
/// or any block-level [TextContentElement] that contains only whitespace and doesn't follow
/// a block element or a line break.
static StyledElement _removeEmptyElements(StyledElement tree) {
List<StyledElement> toRemove = <StyledElement>[];
Set<StyledElement> toRemove = <StyledElement>{};
bool lastChildBlock = true;
tree.children.forEachIndexed((index, child) {
if (child is EmptyContentElement) {
Expand Down
7 changes: 7 additions & 0 deletions lib/src/tree/replaced_element.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ class TextContentElement extends ReplacedElement {
}
}

class LinebreakContentElement extends ReplacedElement {
LinebreakContentElement({
required super.style,
required super.node,
}) : super(name: 'br', elementId: "[[No ID]]");
}

class EmptyContentElement extends ReplacedElement {
EmptyContentElement({required super.node, String name = "empty"})
: super(name: name, style: Style(), elementId: "[[No ID]]");
Expand Down
87 changes: 86 additions & 1 deletion test/test_utils.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import 'dart:async';

import 'package:flutter/material.dart';
import 'package:flutter_html/src/css_box_widget.dart';
import 'package:flutter_html/flutter_html.dart';
import 'package:flutter_test/flutter_test.dart';

class TestApp extends StatelessWidget {
Expand Down Expand Up @@ -141,3 +143,86 @@ CssBoxWidget? findCssBox(Finder finder) {
return found.first.widget as CssBoxWidget;
}
}

Future<StyledElement> generateStyledElementTreeFromHtml(
WidgetTester tester, {
required String html,
bool applyStyleSteps = true,
bool applyProcessingSteps = true,
bool shrinkWrap = false,
List<HtmlExtension> extensions = const [],
Map<String, Style> styles = const {},
}) async {
final completer = Completer<StyledElement>();

await tester.pumpWidget(TestApp(
child: Html(
data: html,
shrinkWrap: shrinkWrap,
extensions: [
...extensions,
TestExtension(
beforeStyleCallback: (tree) {
if (!applyStyleSteps) {
completer.complete(tree);
}
},
beforeProcessingCallback: (tree) {
if (!completer.isCompleted && !applyProcessingSteps) {
completer.complete(tree);
}
},
finalCallback: (tree) {
if (!completer.isCompleted) {
completer.complete(tree);
}
},
),
],
style: styles,
),
));

return completer.future;
}

class TestExtension extends HtmlExtension {
final void Function(StyledElement)? beforeStyleCallback;
final void Function(StyledElement)? beforeProcessingCallback;
final void Function(StyledElement)? finalCallback;

TestExtension({
this.beforeStyleCallback,
this.beforeProcessingCallback,
this.finalCallback,
});

@override
Set<String> get supportedTags => {};

@override
bool matches(ExtensionContext context) {
return context.currentStep != CurrentStep.preparing &&
context.elementName == "html";
}

@override
void beforeStyle(ExtensionContext context) {
beforeStyleCallback?.call(context.styledElement!);
}

@override
void beforeProcessing(ExtensionContext context) {
beforeProcessingCallback?.call(context.styledElement!);
}

@override
InlineSpan build(ExtensionContext context, buildChildren) {
finalCallback?.call(context.styledElement!);
return context.parser.buildFromExtension(
context,
buildChildren,
extensionsToIgnore: {this},
);
}
}
Loading

0 comments on commit cc00406

Please sign in to comment.