Merge pull request #898 from openkraken/fix/sliver-child-text

fix: sliver child is text without renderer should not accpet

integration_tests/specs/css/css-display/sliver.ts

@@ -153,4 +153,26 @@ describe('display sliver', () => {
     await snapshot();
   });
 
+  it('sliver child is text or comment', async () => {
+    var comment = document.createComment('HelloWorld');
+    var text = document.createTextNode('HelloWorld');
+    // Empty text node has different logic in backend.
+    var emptyText = document.createTextNode('');
+
+    var container = createSliverBasicCase();
+
+    container.insertBefore(emptyText, container.firstChild);
+    container.insertBefore(text, container.firstChild);
+    container.insertBefore(comment, container.firstChild);
+    expect(container.childNodes.length).toEqual(103);
+
+    await snapshot(); // Not throws error is ok.
+  });
+
+  it('sliver child is display none', async () => {
+    var container = createSliverBasicCase();
+
+    container.children[2].style.display = 'none';
+    await snapshot(); // Not throws error is ok.
+  });
 });

kraken/lib/dom.dart

@@ -16,6 +16,7 @@ export 'src/dom/text_node.dart';
 export 'src/dom/window.dart';
 export 'src/dom/document.dart';
 export 'src/dom/document_fragment.dart';
+export 'src/dom/sliver_manager.dart';
 
 // Elements
 export 'src/dom/elements/semantics_text.dart';

kraken/lib/src/dom/element.dart

@@ -17,7 +17,6 @@ import 'package:kraken/bridge.dart';
 import 'package:kraken/css.dart';
 import 'package:kraken/dom.dart';
 import 'package:kraken/rendering.dart';
-import 'package:kraken/src/dom/sliver_manager.dart';
 import 'package:meta/meta.dart';
 
 import 'element_native_methods.dart';
@@ -391,7 +390,9 @@ class Element extends Node
   @override
   void willAttachRenderer() {
     // Init render box model.
-    createRenderer();
+    if (renderStyle.display != CSSDisplay.none) {
+      createRenderer();
+    }
   }
 
   @override
@@ -575,9 +576,9 @@ class Element extends Node
   void attachTo(Node parent, {RenderBox? after}) {
     _applyStyle(style);
 
-    if (renderStyle.display != CSSDisplay.none) {
-      willAttachRenderer();
+    willAttachRenderer();
 
+    if (renderer != null) {
       // HTML element override attachTo method to attach renderObject to viewportBox.
       if (parent is Element) {
         RenderLayoutBox? parentRenderLayoutBox = parentElement?._renderLayoutBox?.renderScrollingContent ?? parentElement?._renderLayoutBox;
@@ -1710,9 +1711,9 @@ void _detachRenderBoxModel(RenderBox renderBox) {
   // Remove reference from parent
   RenderObject? parentRenderObject = renderBox.parent as RenderObject;
   if (parentRenderObject is RenderObjectWithChildMixin) {
-    parentRenderObject.child = null; // RenderViewportBox
+    parentRenderObject.child = null; // Case for single child, eg. RenderViewportBox
   } else if (parentRenderObject is ContainerRenderObjectMixin) {
-    parentRenderObject.remove(renderBox); // RenderLayoutBox or RenderSliverList
+    parentRenderObject.remove(renderBox); // Case for multi children, eg. RenderLayoutBox or RenderSliverList
   }
 }
 

kraken/lib/src/dom/sliver_manager.dart

@@ -61,18 +61,21 @@ class RenderSliverElementChildManager implements RenderSliverBoxChildManager {
         throw FlutterError('Sliver unsupported type ${childNode.runtimeType} $childNode');
     }
 
-    assert(child != null, 'Sliver render node should own RenderBox.');
+    // If renderer is not created, use an empty RenderBox to occupy the position, but not do layout or paint.
+    child ??= _createEmptyRenderObject();
 
     if (_hasLayout) {
-      _sliverListLayout
-        ..setupParentData(child!)
-        ..insertSliverChild(child, after: after);
+      _sliverListLayout.insertSliverChild(child, after: after);
     }
 
     childNode.didAttachRenderer();
     childNode.ensureChildAttached();
   }
 
+  RenderBox _createEmptyRenderObject() {
+    return _RenderSliverItemProxy();
+  }
+
   @override
   bool debugAssertChildListLocked() => true;
 
@@ -98,8 +101,8 @@ class RenderSliverElementChildManager implements RenderSliverBoxChildManager {
       }
     }
 
-    // Fallback operation.
-    child.detach();
+    // Fallback operation, remove child from sliver list.
+    _sliverListLayout.remove(child);
   }
 
   @override
@@ -137,3 +140,6 @@ class RenderSliverElementChildManager implements RenderSliverBoxChildManager {
     return trailingScrollOffset + averageExtent * remainingCount;
   }
 }
+
+/// Used for the placeholder for empty sliver item.
+class _RenderSliverItemProxy extends RenderProxyBox {}

kraken/lib/src/rendering/box_model.dart

@@ -140,12 +140,9 @@ mixin RenderBoxContainerDefaultsMixin<ChildType extends RenderBox,
   /// walking the child list directly.
   List<ChildType> getChildren() {
     final List<ChildType> result = <ChildType>[];
-    RenderBox? child = firstChild;
-    while (child != null) {
-      final ParentDataType childParentData = child.parentData as ParentDataType;
+    visitChildren((child) {
       result.add(child as ChildType);
-      child = childParentData.nextSibling;
-    }
+    });
     return result;
   }
 }
@@ -162,7 +159,7 @@ class RenderLayoutBox extends RenderBoxModel
     renderStyle: renderStyle,
   );
 
-  // House content which can be scrolled.
+  // Host content which can be scrolled.
   RenderLayoutBox? get renderScrollingContent {
     if (firstChild is RenderLayoutBox) {
       RenderLayoutBox _firstChild = firstChild as RenderLayoutBox;
@@ -184,8 +181,8 @@ class RenderLayoutBox extends RenderBoxModel
   void markNeedsLayout() {
     super.markNeedsLayout();
 
-    // FlexItem layout must trigger flex container to layout.
-    if (parent != null && parent is RenderFlexLayout) {
+    // FlexItem layout must trigger flex container to relayout.
+    if (parent is RenderFlexLayout) {
       markParentNeedsLayout();
     }
   }