New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 798397 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

[LayoutNG] Calculate negative clearance

Project Member Reported by robho...@gmail.com, Jan 2 2018

Issue description

The first few examples in LayoutTests/fast/block/float/element-clears-float-without-clearance.html require negative clearance in order to pass and it looks like LayoutNG doesn't support negative clearance.

What I think it boils down to is the need for layout to recognise that in this case:

<style>
.float {
    background-color: aqua;
    float: left;
    height: 30px;
}

.clear {
    clear: left;
    margin-top:50px;
    height: 50px;
    background-color: orange;
}
</style>
<div style="overflow:hidden;">
    <div id="normal">
        <div style="float:left;" id="float">float</div>
        <div style="margin-top: 50px; clear:left" id="clear">clear</div>
    </div>
</div>

if #clear did not have 'clear:left;' #float and #clear would have same block offset and so #clear needs to get clearance applied. Since it has a margin of 50px the clearance required to place it after the float is negative, i.e. -20px.

See https://lists.w3.org/Archives/Public/www-style/2014Nov/0297.html for more detailed discussion.

Clearance in LayoutNG doesn't come up with a layout without clearance applied so it doesn't perform this adjustment, it just uses whatever is greater between the margin and the clearance value required to place #clear after #float.

The difficulty here is that 'clear:left;' on #clear affects the initial position of the float, without it #float is affected by the 50px margin, but with it, it is not.

I'm still wrapping my head around LayoutNG so I'm not sure if the appropriate way to fix is this by doing a layout pass without clearance applied to get the initial position of the float and then adjust the clearance on the child in a subquent pass that also positions the float correctly.

@kilpatrick - any thoughts or pointers?




 

Comment 1 by robho...@gmail.com, Jan 2 2018

More info on negative clearance at https://bugs.chromium.org/p/chromium/issues/detail?id=434048
Hey Rob,

Right - LayoutNG currently has Edge's behaviour, it was unclear to me at the time how much of a compat risk this is. If we don't think it'll be much of a compat risk it might be worth keeping the current behaviour, but I'm not sure.

I was thinking yesterday on how to implement this, and it might be fairly straight forward.

This slide deck has a bunch of information about how block layout works. This is one of the key slides.
https://youtu.be/Y5Xa4H2wtVA?t=21m38s

The key idea is that - positions of all the elements bubble up the tree. This is triggered by "clear" as it has enough information to know where it'll get positioned.

The path which this is currently taking is:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc?l=827

The logic roughly is:
 - "float", doesn't get positioned yet, gets added as an "unpositioned float"
 - "clear", pre-layout - flushes all the unpositioned floats and affected by clearance. (line 827).
 - "normal", gets BFCOffset resolved in line 827 block. NGPreviousInflowPosition gets reset. (line 853)
      *** Within this block you may want to add a bit of information that negative clearance should be applied.
          Something like previous_inflow_position->apply_negative_clearance = true, so that when we create the constraints
          for "clear" it knows to follow this rule.
          Or might be better to set a bool here, and pass into ComputeChildData, not sure :/, depends on if this needs to be propagated.
      ***
 - "clear", gets laid out, positions itself (I think this will occur due to is_non_empty_inline check around line 831).
     - It's actual BFCOffset will get set within: blink::NGBlockLayoutAlgorithm::MaybeUpdateFragmentBfcOffset, which adheres to the clearance line passed in. If we need to pass an extra bit of information into "clear" to properly handle -ve clearance, then this would be the correct spot to perform this check and adjustment.

To start debugging the above I'd set a breakpoint on:

NGBlockLayoutAlgorithm::HandleFloat (see it being added as an unpositioned float), and then NGBlockLayoutAlgorithm::HandleInflow

It would be good to check what happens with more complex margin struts as well. E.g. adding things like

<div style="margin-top: -40px">
  <div style="margin-bottom: 50px"></div>
</div>

In various places (after float, at start of "clear") to make sure we get the right propagation of the input bit, etc. 
Components: Blink>Layout

Sign in to add a comment