A view referencing a View that's not a peer (same superview) in Dim/Pos should not be allowed #2461
Description
I was debugging tests and came across this test:
public void DimCombine_Do_Not_Throws ()
{
Application.Init (new FakeDriver ());
var t = Application.Top;
var w = new Window ("w") {
Width = Dim.Width (t) - 2,
Height = Dim.Height (t) - 2
};
var f = new FrameView ("f");
var v1 = new View ("v1") {
Width = Dim.Width (w) - 2,
Height = Dim.Height (w) - 2
};
var v2 = new View ("v2") {
Width = Dim.Width (v1) - 2,
Height = Dim.Height (v1) - 2
};
f.Add (v1, v2);
w.Add (f);
t.Add (w);
f.Width = Dim.Width (t) - Dim.Width (v2);
f.Height = Dim.Height (t) - Dim.Height (v2);
t.Ready += (s, e) => {
Assert.Equal (80, t.Frame.Width);
Assert.Equal (25, t.Frame.Height);
Assert.Equal (78, w.Frame.Width);
Assert.Equal (23, w.Frame.Height);
Assert.Equal (6, f.Frame.Width);
Assert.Equal (6, f.Frame.Height);
Assert.Equal (76, v1.Frame.Width);
Assert.Equal (21, v1.Frame.Height);
Assert.Equal (74, v2.Frame.Width);
Assert.Equal (19, v2.Frame.Height);
};
Application.Iteration += () => Application.RequestStop ();
Application.Run ();
Application.Shutdown ();
}I have decided this test is totally bogus because views are referencing superviews.
This code would guard against this:
internal void CollectPos (Pos pos, View from, ref HashSet<View> nNodes, ref HashSet<(View, View)> nEdges)
{
switch (pos) {
case Pos.PosView pv:
if (!from.InternalSubviews.Contains (pv.Target)) {
throw new InvalidOperationException ($"View {pv.Target} is not a subview of {from}");
}
if (pv.Target != this) {
nEdges.Add ((pv.Target, from));
}
foreach (var v in from.InternalSubviews) {
CollectAll (v, ref nNodes, ref nEdges);
}
return;
case Pos.PosCombine pc:
foreach (var v in from.InternalSubviews) {
CollectPos (pc.left, from, ref nNodes, ref nEdges);
CollectPos (pc.right, from, ref nNodes, ref nEdges);
}
break;
}
}
internal void CollectDim (Dim dim, View from, ref HashSet<View> nNodes, ref HashSet<(View, View)> nEdges)
{
switch (dim) {
case Dim.DimView dv:
if (!from.InternalSubviews.Contains (dv.Target)) {
throw new InvalidOperationException ($"View {dv.Target} is not a subview of {from}");
}
if (dv.Target != this) {
nEdges.Add ((dv.Target, from));
}
foreach (var v in from.InternalSubviews) {
CollectAll (v, ref nNodes, ref nEdges);
}
return;
case Dim.DimCombine dc:
foreach (var v in from.InternalSubviews) {
CollectDim (dc.left, from, ref nNodes, ref nEdges);
CollectDim (dc.right, from, ref nNodes, ref nEdges);
}
break;
}
}And this test would verify:
[Fact]
public void Dim_Referencing_SuperView_Throws ()
{
var super = new View ("super") {
Width = 10,
Height = 10
};
var view = new View ("view") {
Width = Dim.Width (super), // this is not allowed
Height = Dim.Height (super), // this is not allowed
};
super.Add (view);
super.BeginInit ();
super.EndInit ();
Assert.Throws<InvalidOperationException> (() => super.LayoutSubviews ());
}This breaks a lot of existing code (e.g. TileView does this internally). So I'm not sure I'm correct.
Please debate ;-).
Thanks.