1485764 - be more precise about not placing floats until a breaking opportunity

Status: NEW

(Core :: Layout: Floats, enhancement)

Description

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I think taking the approach from bug 488725 even more than we currently are would probably fix some subtle float handling bugs.

In particular, there's no reason that the approach bug 488725 takes should be specific to no-wrap at all.  Floats can be anchored in the middle of a word -- at a non-breakable point -- and all of the concerns that apply to nowrap apply to that case as well.


I'd also note that the code in nsLineLayout::EndSpan that does:

  if (psd->mNoWrap && !psd->mParent->mNoWrap) {
    FlushNoWrapFloats();
  }

isn't quite correct even that way, since there isn't necessarily a breaking opportunity there, e.g., if it the markup is:

  This is <span nowrap>a bit <span float></span> of a long</span>wordexample.

then we really shouldn't try to place the float until after the period, not at the last </span>.

Running the code in NotifyOptionalBreakPosition whether or not mCurrentSpan->mNoWrap would allow removing that code, which really exists primarily because the code in NotifyOptionalBreakPosition is conditional.


(That said, it's possible that the line redo mechanism with the remembered break position part of NotifyOptionalBreakPosition saves us in some of these cases.)

Comment 1

[Emilio Cobos Álvarez [:emilio]]

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-7 from comment #0)
> I think taking the approach from bug 488725 even more than we currently are
> would probably fix some subtle float handling bugs.
> 
> In particular, there's no reason that the approach bug 488725 takes should
> be specific to no-wrap at all.  Floats can be anchored in the middle of a
> word -- at a non-breakable point -- and all of the concerns that apply to
> nowrap apply to that case as well.
> 
> 
> I'd also note that the code in nsLineLayout::EndSpan that does:
> 
>   if (psd->mNoWrap && !psd->mParent->mNoWrap) {
>     FlushNoWrapFloats();
>   }
> 
> isn't quite correct even that way, since there isn't necessarily a breaking
> opportunity there, e.g., if it the markup is:
> 
>   This is <span nowrap>a bit <span float></span> of a long</span>wordexample.
> 
> then we really shouldn't try to place the float until after the period, not
> at the last </span>.

Hmm, I see I thought we really should, since we usually consider placing a float in non-nowrap contexts itself a breaking opportunity:

  https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/layout/generic/nsLineLayout.cpp#1123

But I guess the really important bit is the !continuingTextRun bit. Though nsPlaceHolderFrame::CanContinueTextRun forwards to the OOF, which looks rather dubious...

Anyway, that's the main reason I wrote the patch the way I did, to avoid changing that. I guess this should be reasonable though... Does something about what I mentioned above change your mind about it? If not, the patch shouldn't be really hard...

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

I guess this requires (a) some research into what other browsers do and then (b) getting the CSS WG to clarify the spec to say whether or not float placeholders constitute line-breaking opportunities.  I've always thought they shouldn't be since the float is supposed to be removed from the flow... but I guess this stuff has been around long enough that we probably can't apply logic to it anymore.

Comment 3

[Emilio Cobos Álvarez [:emilio]]

In https://bugs.chromium.org/p/chromium/issues/detail?id=906369 some Blink engineers and me are discussing related things, about whether OOFs should constitute breaking opportunities. Looks like Edge doesn't consider them breaking opportunities already, so we may have some leeway:

  https://bugs.chromium.org/p/chromium/issues/detail?id=906369#c14

Comment 4

[Florian Rivoal]

getting the CSS WG to clarify the spec to say whether or not float placeholders constitute line-breaking opportunities

css-text-3 Section 5.1 says:

Out-of-flow elements do not introduce a forced line break or soft wrap opportunity in the flow.

CSS2 section 16.6.1 agrees:

Floated and absolutely-positioned elements do not introduce a line breaking opportunity.

So you should be in the clear.