Bug 1493962 Comment 11 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Review of attachment 9040484 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsFlexContainerFrame.cpp
@@ +1321,5 @@
>    // --------------------------
>    float flexGrow, flexShrink;
>    if (IsLegacyBox(this)) {
> +    nscoord flex;
> +    nsFrame::AddXULFlex(aChildFrame, flex);

You don't want to make this call, if aChildFrame happens to be an anonymous flex item.  This is what's causing the issues that Brian mentioned in comment 9. (At least the first issue with the URLbar, and probably the second as well.)

If aChildFrame is an anonymous flex item, e.g. a wrapper for text or for an abspos placeholder, then we want it to just have default sizing properties -- but this change will make it potentially read flex information off of the associated DOM node (which I think is actually the flex *container*'s DOM node).

So: you probably want something like
 if (aChildFrame->Style()->IsAnonBox()) {
   flex = aFrame->Style()->IsAnonBox();
 } else {
   nsFrame::AddXULFlex(aChildFrame, flex);
 }

This is probably true for any other places in this patch where you read DOM attributes for a flex item.  If the flex item is anonymous, then its DOM node is the flex container itself, which isn't what you want to be reading flex-item attributes from.
Review of attachment 9040484 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsFlexContainerFrame.cpp
@@ +1321,5 @@
>    // --------------------------
>    float flexGrow, flexShrink;
>    if (IsLegacyBox(this)) {
> +    nscoord flex;
> +    nsFrame::AddXULFlex(aChildFrame, flex);

You don't want to make this call, if aChildFrame happens to be an anonymous flex item.  This is what's causing the issues that Brian mentioned in comment 9. (At least the first issue with the URLbar, and probably the second as well.)

If aChildFrame is an anonymous flex item, e.g. a wrapper for text or for an abspos placeholder, then we want it to just have default sizing properties -- but this change will make it potentially read flex information off of the associated DOM node (which I think is actually the flex *container*'s DOM node).

So: you probably want something like
 if (aChildFrame->Style()->IsAnonBox()) {
   flex = ChildFrame->StyleXUL()->mBoxFlex;
 } else {
   nsFrame::AddXULFlex(aChildFrame, flex);
 }

This is probably true for any other places in this patch where you read DOM attributes for a flex item.  If the flex item is anonymous, then its DOM node is the flex container itself, which isn't what you want to be reading flex-item attributes from.

Back to Bug 1493962 Comment 11