OK, so I think this is happening because reflowing the popup frame at bogus position "0,0", via the `LogicalPoint(childWM)` arg here (and only setting the position later on): ``` ReflowChild(aChild, pc, childSize, childRI, childWM, LogicalPoint(childWM), dummyContainerSize, ReflowChildFlags::Default, aStatus); ``` https://searchfox.org/mozilla-central/rev/8bc24752246aeac8a9aed566cf1caccf88d97d11/layout/generic/nsGridContainerFrame.cpp#6438 This is supposed to be fine, because we determine the correct position and pass it in via `FinishReflowChild` down below -- but in fact, it ends up causing trouble because `nsMenuPopupFrame` determines its size (during reflow) **based on its presumed position**. So if we reflow it at the wrong position, we end up with a bogus size for the popup and its descendants. We do end up correcting the nsMenuPopupFrame's own size eventually, via the post-reflow callback `nsMenuPopupFrame::ReflowFinished()`, but that doesn't handle resizing the popup's contents -- it only resizes the popup itself (as part of repositioning it). So the contents still end up being the wrong size, which is why Alice0775's shows them being a bit too small. The simplest solution in this case is to "guess" the correct position and pass it in to reflow -- i.e. to "guess" that the child is at the default position, which is the grid area's origin. We can do this as long as the child and the parent have the same writing mode, which is probably always the case in our XUL code. (If they did disagree on writing mode, then we woudln't be able to reliably express the origin in terms of the child's writing mode until after we've reflowed the child and know its size -- so there'd be a chicken-and-egg problem. But anyway, that's not a problem here.) If we "guess" wrong, then it's generally fine (for the same reason that our current bogus 0,0 point is fine) except in this case of nsMenuPopupFrame which is already broken, so it'd be a low-stakes change. So I think we should just make that additional change to the current patch.
Bug 1593060 Comment 29 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
OK, so I think this is happening because reflowing the popup frame at bogus position "0,0", via the `LogicalPoint(childWM)` arg here (and only setting the position later on): ``` ReflowChild(aChild, pc, childSize, childRI, childWM, LogicalPoint(childWM), dummyContainerSize, ReflowChildFlags::Default, aStatus); ``` https://searchfox.org/mozilla-central/rev/8bc24752246aeac8a9aed566cf1caccf88d97d11/layout/generic/nsGridContainerFrame.cpp#6438 This is supposed to be fine, because we determine the correct position and pass it in via `FinishReflowChild` down below -- but in fact, it ends up causing trouble because `nsMenuPopupFrame` determines its size (during reflow) **based on its presumed position**. So if we reflow it at the wrong position, we end up with a bogus size for the popup and its descendants. We do end up correcting the nsMenuPopupFrame's own size eventually, via the post-reflow callback `nsMenuPopupFrame::ReflowFinished()`, but that doesn't handle resizing the popup's contents -- it only resizes the popup itself (as part of repositioning it). So the contents still end up being the wrong size, which is why Alice0775's screenshot shows them being a bit too small. The simplest solution in this case is to "guess" the correct position and pass it in to reflow -- i.e. to "guess" that the child is at the default position, which is the grid area's origin. We can do this as long as the child and the parent have the same writing mode, which is probably always the case in our XUL code. (If they did disagree on writing mode, then we woudln't be able to reliably express the origin in terms of the child's writing mode until after we've reflowed the child and know its size -- so there'd be a chicken-and-egg problem. But anyway, that's not a problem here.) If we "guess" wrong, then it's generally fine (for the same reason that our current bogus 0,0 point is fine) except in this case of nsMenuPopupFrame which is already broken, so it'd be a low-stakes change. So I think we should just make that additional change to the current patch.