1283222 - Unwanted line-break opportunities at out-of-flow frames [was: Commas wrapped to beginning of line with text-align: justify]

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[Kevin Scannell]

Attachment: skeealaght-hover.html

I'm attaching a minimal test case which shows the problem; a comma which I'd expect to render at the end of a line is wrapping to the beginning of the next line.  I've seen the same behavior with . ! ? etc. 

The HTML may look strange; it's intended to show the nested <span> as a mouseover tooltip, but I removed all of that CSS since it's not relevant for reproducing the bug.  My guess is that the code for text-align: justify does check for trailing punctuation but is being tricked by these nested spans.

This renders correctly in Chrome 51.0.2704.103 and Safari 9.1.1 (11601.6.17)

Comment 1

[Jonathan Kew [:jfkthame]]

I don't think this is actually related to text-align:justify; I see the same undesired line-break if I remove that declaration and leave the paragraph left-aligned.

Comment 2

[Jonathan Kew [:jfkthame]]

In general, it appears that where there's an out-of-flow frame like a float or abs-pos element, we allow a line-break. This isn't necessarily an appropriate place for a break to happen.

Reduced testcase:

data:text/html,<div style="width:1em">hello<div style="float:right"></div>, cruel<div style="position:absolute"></div>, world

This has line-breaks before both the commas.

Comment 4

[Jeremy Chen ([e-mail])]

(In reply to Jonathan Kew (:jfkthame) from comment #2)
> In general, it appears that where there's an out-of-flow frame like a float
> or abs-pos element, we allow a line-break. This isn't necessarily an
> appropriate place for a break to happen.
> 
> Reduced testcase:
> 
> data:text/html,<div style="width:1em">hello<div style="float:right"></div>,
> cruel<div style="position:absolute"></div>, world
> 
> This has line-breaks before both the commas.

Looks like this reduced testcase is rendered exactly the same on Firefox Nightly (59.0a1), Safari 11 (12604.1.38.1.7), and Chrome 62.0.3202.94. However, the testcase in comment 0 still sustains, i.e., good in Chrome, Safari and bad in Firefox.

Since the testcase in comment 0 is pretty complicated, I'll start working on a reduced testcase and see if I can make some progress here.

Comment 5

[Jeremy Chen ([e-mail])]

Attachment: reduced-unwanted-linebreak-testcase.html

This is reduced from the testcase in comment 0. The dom/frame structure is much cleaner now.

Comment 6

[Jeremy Chen ([e-mail])]

Attachment: reduced-unwanted-linebreak-testcase.html

This is reduced from the testcase in comment 0, but dom/frame structure is much cleaner.

Looks like the out-of-flow <span> would give an extra line-break opportunity before the comma punctuation, even the <span> is actually empty.

From the clues so far, I think the testcase in comment 2 should be able to reproduce the issue, but it just can't (i.e., testcase in comment 2 renders the same in Chrome, Safari, and Firefox)....

Comment 7

[Jeremy Chen ([e-mail])]

I think the out-of-flow <span> is treated as a block frame, which should presumably separate the texts and create a break opportunity. This, which is the current behavior in Firefox, does make sense to me. What I can't understand is, the rendering results of both testcases (comment 2 and comment 6) are the same in Firefox; whereas the rendering results of both testcases are different in Chrome & Safari.

Before going further, It'd be better to figure out the definition about what the correct behavior that we/developers/spec editors expect first. Then we can discuss about the pros vs. cons. To me, I can only think of either allow or disallow line-break opportunities at out-of-flow frames. But either way, I would assume the behavior would be still the same for both testcases (comment 2 and comment 6) in Firefox, which means we will still be different from Chrome & Safari either for testcase in comment 2 or for testcase in comment 6.

That said, no matter we should allow line-break opportunities at out-of-flow frames or not (not sure which spec I should check for the behavior), the disagreement of testcases (comment2 and comment 6) seems to be a bug for Chrome & Safari.

Hi Jonathan, is there something that I might miss? or, any direction that I could further dig into?

Comment 8

[Jonathan Kew [:jfkthame]]

I think in general an out-of-flow element should not introduce a line-break opportunity where one would not otherwise exist in the text; the fact that it's out-of-flow ought to make the element "invisible" to the processing of the normal (in-flow) text around it.

Regarding the Safari/Chrome behavior on the comment 2 testcase, I think this is a different case: there, the block width is so small that the individual words already overflow the available width. In this case, Safari and Chrome may allow the "unexpected" break at the out-of-flow element, but they don't do this otherwise (when the block width is more normal, so that multiple words occur on each line and an earlier line-break would be available). Actually, Safari and Chrome differ slightly from each other here, too, in that Chrome may also allow such a break if there is an *in-flow* empty <span> (which I think is a bug), whereas Safari doesn't.

I'll attach a further testcase that I think makes things clearer.

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: additional testcase for cross-browser comparison

In particular, the last example in the left-hand column shows Firefox giving a result that is inferior to the other browsers. This is the example that's analogous to the original report here.

Comparing with Chrome and Safari shows that in Chrome, the factor that affects breaking is the presence/absence of the empty spans, and not whether they are in- or out-of-flow; in Safari, the out-of-flow (but not in-flow) spans provide a break opportunity but only if no earlier break possibility existed on the line.

Comment 10

[Jeremy Chen ([e-mail])]

Thank you for the example, I have a more clear view now.

That said, assume we prefer not to introduce a line-break opportunity for out-of-flow element, take following definitions for example:

wordBeforeOOF: a word that is right before an out-of-flow element
OOFElement: the out-of-flow element, could be empty or not
puncAfterOOF: a punctuation that is right after an out-of-flow element

We can either:
A. look ahead for the possible OOFElement while processing wordBeforeOOF
B. look back while processing OOFElement

As for plan A, I think this definitely hurts our performance, and IIRC we used to utilize a look ahead algorithm for texts but eventually removed that algorithm (due to performance issue or something, I can't remember the bug number).

So, I tried a little bit hack for plan B by allowing continuations for OOFElements. Here's the try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b941818f4add93228842fe26492b77e4551cf683&selectedJob=146717254

The failures about white-space shifting may not be an issue. It is the css/CSS2/floats-clear/floats-006.xht that I have concern for, since the rendering result of float elements is different.

Even if we can solve all these failures, I suspect we might still need to take care of the case "the block width is so small". Otherwise, we'll get a very different result from Chrome/Safari.

Comment 11

[Jonathan Kew [:jfkthame]]

(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #10)
> Thank you for the example, I have a more clear view now.
> 
> That said, assume we prefer not to introduce a line-break opportunity for
> out-of-flow element, take following definitions for example:
> 
> wordBeforeOOF: a word that is right before an out-of-flow element
> OOFElement: the out-of-flow element, could be empty or not
> puncAfterOOF: a punctuation that is right after an out-of-flow element
> 
> We can either:
> A. look ahead for the possible OOFElement while processing wordBeforeOOF
> B. look back while processing OOFElement
> 
> As for plan A, I think this definitely hurts our performance, and IIRC we
> used to utilize a look ahead algorithm for texts but eventually removed that
> algorithm (due to performance issue or something, I can't remember the bug
> number).
> 
> So, I tried a little bit hack for plan B by allowing continuations for
> OOFElements. Here's the try run:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=b941818f4add93228842fe26492b77e4551cf683&selectedJob=1
> 46717254

I think the basic idea that placeholder frames should in general be ignorable here is right, but this may not be quite the right place for it. The issue isn't primarily about whether a textrun can be continued, but how this interacts with finding line-break boundaries. So I suspect the patch may need to be somewhere more like BuildTextRunsScanner, where we are collecting text to be fed into the line-breaking code.

> 
> The failures about white-space shifting may not be an issue. It is the
> css/CSS2/floats-clear/floats-006.xht that I have concern for, since the
> rendering result of float elements is different.

I think this actually has the same root cause as the other whitespace failures: the problem is that white-space collapsing isn't happening across the out-of-flow element. If you remove all the whitespace from between the two floated <div>s in the floats-006 testcase, it will no longer fail, but as soon as there is a space (which no longer collapses) there, they don't fit on the same line any more.

(Of course, that's not a solution, it's just to demonstrate that it is this whitespace that leads to the failure.)

> 
> Even if we can solve all these failures, I suspect we might still need to
> take care of the case "the block width is so small". Otherwise, we'll get a
> very different result from Chrome/Safari.

I'm not too worried about getting a different result from Chrome/Safari here; note that they don't agree with each other either, and Edge is different yet again. Clearly, there's no interop on this issue, but at the same time, our current behavior gives visibly poor results.

It might be worth trying to discuss with the other browser developers, and see if some agreement can be reached, but IIRC the details of line-breaking are left somewhat implementation-defined, so it's possible there is not currently any spec that gives clear guidance here.

The most sensible behavior, I believe, is that both an empty inline element <span></span> and an out-of-flow element (e.g. float or abs-pos) should be ignored as far as line-breaking is concerned. That's what Edge does, according to the testcase; and it's what Firefox and Safari both do for the empty inline element. Chrome, OTOH, allows a break (but only if the line has *already* overflowed), regardless of whether the empty element is inline or out-of-flow. I think that's simply a Blink bug; and I think it's a Webkit bug that they allow out-of-flows to introduce line-break positions.

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: updated testcase

Here's a further minor update to my testcase.

IE11, Edge: (A), (B) and (C) all render the same, with no extra line-breaks. I believe this is the most "correct" behavior.

Chrome: (A) and (B) render the same, with unexpected line-breaks at the empty <span>s whether they are in- or out-of-flow, but only if the text has already *overflowed* the box (not when the next character will overflow, so it doesn't break "linebreaking" in [2] or later, nor before the first two commas in [3]).

Safari: (B) and (C) render the same (correct), while (A) gets unexpected line-breaks, similarly (but not identically) to Chrome's behavior with (A) and (B). It seems to use the out-of-flow as a break opportunity when it occurs *at* the edge of the box, not only when it has already overflowed *past* the edge. (So it breaks "linebreaking" in [2] but not in [3].) But it doesn't go back and break at an out-of-flow earlier in the line, even if there's no other break opportunity.

Firefox: (B) and (C) render the same (correct), while (A) gets unexpected line-breaks, slightly differently from either Chrome or Safari. (We treat all the out-of-flows as line-break opportunities, whether or not the line has already overflowed, and whether or not there are other breaks.)

If we can fix this bug such that we end up matching Edge here, I'll be happy. :) Authors who want more breaks should use the word-break or overflow-wrap properties, or add appropriate controls such as zero-width space in their content.

Comment 13

[Jonathan Kew [:jfkthame]]

Actually, checking [1], although "CSS does not fully define where soft wrap opportunities occur", it does explicitly mention that "out-of-flow elements do not introduce a forced line break or soft wrap opportunity in the flow".

This supports my position that in terms of the effect of the markup and properties here, IE11/Edge is correct; our behavior on column (A) is wrong, as is Safari's (in a similar but not-quite-identical way); and Chrome in addition shows buggy behavior in column (B).

So fixing this is required for CSS Text 3 conformance. And adding a relevant web-platform test, if one doesn't already exist (presumably not, or we'd be failing it!) would be good.

[1] https://drafts.csswg.org/css-text-3/#line-break-details

Comment 14

[Jeremy Chen ([e-mail])]

(In reply to Jonathan Kew (:jfkthame) from comment #11)
> 
> I think the basic idea that placeholder frames should in general be
> ignorable here is right, but this may not be quite the right place for it.
> The issue isn't primarily about whether a textrun can be continued, but how
> this interacts with finding line-break boundaries. So I suspect the patch
> may need to be somewhere more like BuildTextRunsScanner, where we are
> collecting text to be fed into the line-breaking code.

Thanks for the feedback of the concept. I'll look into BuildTextRunsScanner.

> 
> I'm not too worried about getting a different result from Chrome/Safari
> here; note that they don't agree with each other either, and Edge is
> different yet again. Clearly, there's no interop on this issue, but at the
> same time, our current behavior gives visibly poor results.
> 
> It might be worth trying to discuss with the other browser developers, and
> see if some agreement can be reached, but IIRC the details of line-breaking
> are left somewhat implementation-defined, so it's possible there is not
> currently any spec that gives clear guidance here.
> 
> The most sensible behavior, I believe, is that both an empty inline element
> <span></span> and an out-of-flow element (e.g. float or abs-pos) should be
> ignored as far as line-breaking is concerned. That's what Edge does,
> according to the testcase; and it's what Firefox and Safari both do for the
> empty inline element. Chrome, OTOH, allows a break (but only if the line has
> *already* overflowed), regardless of whether the empty element is inline or
> out-of-flow. I think that's simply a Blink bug; and I think it's a Webkit
> bug that they allow out-of-flows to introduce line-break positions.

Ok, I'll keep digging toward this direction.

(In reply to Jonathan Kew (:jfkthame) from comment #12)
> Created attachment 8931416 [details]
> updated testcase
> 
> Here's a further minor update to my testcase.
> 
> IE11, Edge: (A), (B) and (C) all render the same, with no extra line-breaks.
> I believe this is the most "correct" behavior.
> 
> Chrome: (A) and (B) render the same, with unexpected line-breaks at the
> empty <span>s whether they are in- or out-of-flow, but only if the text has
> already *overflowed* the box (not when the next character will overflow, so
> it doesn't break "linebreaking" in [2] or later, nor before the first two
> commas in [3]).
> 
> Safari: (B) and (C) render the same (correct), while (A) gets unexpected
> line-breaks, similarly (but not identically) to Chrome's behavior with (A)
> and (B). It seems to use the out-of-flow as a break opportunity when it
> occurs *at* the edge of the box, not only when it has already overflowed
> *past* the edge. (So it breaks "linebreaking" in [2] but not in [3].) But it
> doesn't go back and break at an out-of-flow earlier in the line, even if
> there's no other break opportunity.
> 
> Firefox: (B) and (C) render the same (correct), while (A) gets unexpected
> line-breaks, slightly differently from either Chrome or Safari. (We treat
> all the out-of-flows as line-break opportunities, whether or not the line
> has already overflowed, and whether or not there are other breaks.)
> 
> If we can fix this bug such that we end up matching Edge here, I'll be
> happy. :) Authors who want more breaks should use the word-break or
> overflow-wrap properties, or add appropriate controls such as zero-width
> space in their content.

Yeah, I personally prefer Edge's result as well. The previous WIP let us match Edge too.
Thank you for pointing out the spec words, that gives me more confidence to do so.
So, I'll stick with the concept, and see if I can come up with a fine implementation.

Comment 15

[Jeremy Chen ([e-mail])]

I took a look at BuildTextRunsScanner and some text building pipeline nearby, two approaches came up in my mind:

(a) We could continue the textrun for OOF frames,

which seems okay for OOF empty frames. But I'm not convinced that the OOF non-empty frames could be handled properly as well. Some complicated/hacky process might need to be added to ensure the style of the OOF frames are correct, since OOF frames are expected to have different style data from their in-flow siblings.

(b) Or, we could keep the textrun structures as they are, but suppress the linebreak opportunity before/after an OOF frame.

This seems promising. But I'm not sure if we can force suppress the linebreak for textrun boundaries?



Another choice could be (I guess), just go with plan (a) and only take care of OOF empty frames in this bug?

Well, since I believe the wonderful world would be handle all the OOF frames in one same reasonable logic, I'll keep digging for plan (b).

Comment 16

[Jonathan Kew [:jfkthame]]

I wondered, does it work to ignore placeholder frames by just returning FB_CONTINUE at the beginning of BuildTextRunsScanner::FindBoundaries, in the same way as ruby text container frames are skipped?

I don't think it should make any difference whether the OOF frame is empty or not, as the point is that even if it has lots of content, that content is taken out of the main flow and therefore doesn't affect its layout (including its line-breaking behavior).

Comment 17

[Jeremy Chen ([e-mail])]

(In reply to Jonathan Kew (:jfkthame) from comment #16)
> I wondered, does it work to ignore placeholder frames by just returning
> FB_CONTINUE at the beginning of BuildTextRunsScanner::FindBoundaries, in the
> same way as ruby text container frames are skipped?

Looks like we always early return while finding a line to start building textruns in https://searchfox.org/mozilla-central/rev/ba2b0cf4d16711d37d4bf4d267b187c9a27f6638/layout/generic/nsTextFrame.cpp#1549, so never get the chance to get into BuildTextRunsScanner::FindBoundaries (I use the testcase in comment 12 for verification). Might need some special handling to let BuildTextRunsScanner::FindBoundaries get called in this case. 

> I don't think it should make any difference whether the OOF frame is empty
> or not, as the point is that even if it has lots of content, that content is
> taken out of the main flow and therefore doesn't affect its layout
> (including its line-breaking behavior).

Agree.

Comment 18

[Jeremy Chen ([e-mail])]

(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #17)
> (In reply to Jonathan Kew (:jfkthame) from comment #16)
> > I wondered, does it work to ignore placeholder frames by just returning
> > FB_CONTINUE at the beginning of BuildTextRunsScanner::FindBoundaries, in the
> > same way as ruby text container frames are skipped?
> 
> Looks like we always early return while finding a line to start building
> textruns in
> https://searchfox.org/mozilla-central/rev/
> ba2b0cf4d16711d37d4bf4d267b187c9a27f6638/layout/generic/nsTextFrame.cpp#1549,
> so never get the chance to get into BuildTextRunsScanner::FindBoundaries (I
> use the testcase in comment 12 for verification). Might need some special
> handling to let BuildTextRunsScanner::FindBoundaries get called in this
> case. 

I re-checked the frame tree, and realized that the textrun for in-flow texts are never broken/cut by the OOF frames in the current implementation. Take the (A)-1 example in the testcase from comment 12 as an example, all the texts inside the first <li> element belong to the same textrun. The frame tree dump is attached below, and the "[run length]" part is added by myself to get a more clear view of the textrun status.

I also tried to add some texts for the OOF frames, and it seems the textrun for the in-flow texts is still not broken/cut by the OOF frames. Though the texts for the OOF frame would create a separate textrun for itself, this doesn't affect/break the in-flow textrun.

IIUC, BuildTextRunsScanner::FindBoundaries is called for finding a textrun boundary. Since the in-flow textrun is never separated in this case, I guess I shall keep focus on how to correctly set linebreaks for the in-flow textrun.


```
Block(ol)(0)@129c28680 parent=129c27d78 {0,720,19720,33420} [state=0000160800d00310] [content=129bfc670] [sc=129c27cd0]<
                line 129c430d8: count=2 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:linebr[0x80100] {2400,0,14257,750} vis-overflow=2370,0,14316,750 scr-overflow=2400,0,14257,750 <
                  Text(0)"\n(A) With out-of-flow empty spans:"@129c32a20 parent=129c28680 next=129c32aa8 {2400,0,14256,750} vis-overflow=-30,0,14316,750 scr-overflow=0,0,14256,750 [state=02000048b0600000] [content=18a13a280] [sc=129c28868:-moz-text] [run=12a9c71f0][run length=34][0,34,T] 
                  Frame(br)(1)@129c32aa8 parent=129c28680 next=129c32b18 {16656,0,1,750} [state=0200000800000000] [content=129bfc700] [sc=129c28c50]
                >
                line 129c43128: count=1 state=block,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x108] bm=720 {4132,1470,1419,6870} vis-overflow=2836,1470,4842,6870 scr-overflow=2836,1470,4812,6870 <
                  Block(li)(3)@129c32b18 parent=129c28680 next=129c352f8 {4132,1470,1419,6870} vis-overflow=-1296,0,4842,6870 scr-overflow=-1296,0,4812,6870 [state=0200124840100200] [content=129bfc790] [sc=129c28e18]<
                    line 129c33cf0: count=2 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x520] {60,60,2160,750} vis-overflow=-1296,60,3546,750 scr-overflow=-1296,60,3516,750 <
                      Text(0)"\nhello"@129c33420 parent=129c32b18 next=129c33558 {60,60,2160,750} vis-overflow=-30,0,2220,750 scr-overflow=0,0,2160,750 [state=02000048b0600000] [content=18a13a480] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][0,6,T] 
                      Placeholder(span)(1)@129c33558 parent=129c32b18 next=129c335c8 {2220,630,0,0} [state=0000000000200000] [content=129bfc820] [sc=129c28730:-moz-oof-placeholder^0] outOfFlowFrame=Block(span)(1)@129c334a8
                    >
                    line 129c43868: count=1 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {60,810,432,750} vis-overflow=30,810,924,750 scr-overflow=60,810,432,750 <
                      Text(2)", "@129c335c8 parent=129c32b18 next=129c43788 next-in-flow=129c43788 {60,810,432,750} vis-overflow=-30,0,924,750 scr-overflow=0,0,432,750 [state=02000048b1600000] [content=18a13a580] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][0,2,F] 
                    >
                    line 129c43818: count=2 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {60,1560,2160,750} vis-overflow=30,1560,2220,750 scr-overflow=60,1560,2160,750 <
                      Text(2)"cruel"@129c43788 parent=129c32b18 next=129c33700 prev-in-flow=129c335c8 {60,1560,2160,750} vis-overflow=-30,0,2220,750 scr-overflow=0,0,2160,750 [state=0200004890600004] [content=18a13a580] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][2,5,T] 
                      Placeholder(span)(3)@129c33700 parent=129c32b18 next=129c33770 {2220,2130,0,0} [state=0000000000200000] [content=129bfc8b0] [sc=129c28730:-moz-oof-placeholder^0] outOfFlowFrame=Block(span)(3)@129c33650
                    >
                    line 129c43a20: count=1 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {60,2310,432,750} vis-overflow=30,2310,924,750 scr-overflow=60,2310,432,750 <
                      Text(4)", "@129c33770 parent=129c32b18 next=129c43940 next-in-flow=129c43940 {60,2310,432,750} vis-overflow=-30,0,924,750 scr-overflow=0,0,432,750 [state=02000048b1600000] [content=18a13a600] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][0,2,F] 
                    >
                    line 129c439d0: count=2 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {60,3060,1728,750} vis-overflow=30,3060,1788,750 scr-overflow=60,3060,1728,750 <
                      Text(4)"line"@129c43940 parent=129c32b18 next=129c338a8 prev-in-flow=129c33770 {60,3060,1728,750} vis-overflow=-30,0,1788,750 scr-overflow=0,0,1728,750 [state=0200004890600004] [content=18a13a600] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][2,4,T] 
                      Placeholder(span)(5)@129c338a8 parent=129c32b18 next=129c33918 {1788,3630,0,0} [state=0000000000200000] [content=129bfc940] [sc=129c28730:-moz-oof-placeholder^0] outOfFlowFrame=Block(span)(5)@129c337f8
                    >
                    line 129c43b50: count=2 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {60,3810,3456,750} vis-overflow=30,3810,3516,750 scr-overflow=60,3810,3456,750 <
                      Text(6)"breaking"@129c33918 parent=129c32b18 next=129c33a50 {60,3810,3456,750} vis-overflow=-30,0,3516,750 scr-overflow=0,0,3456,750 [state=02000048b0600000] [content=18a13a700] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][0,8,T] 
                      Placeholder(span)(7)@129c33a50 parent=129c32b18 next=129c33ac0 {3516,4380,0,0} [state=0000000000200000] [content=129bfc9d0] [sc=129c28730:-moz-oof-placeholder^0] outOfFlowFrame=Block(span)(7)@129c339a0
                    >
                    line 129c43ba0: count=1 state=inline,clean,prevmarginclean,not impacted,wrapped,before:nobr,after:nobr[0x120] {60,4560,432,750} vis-overflow=30,4560,924,750 scr-overflow=60,4560,432,750 <
                      Text(8)", "@129c33ac0 parent=129c32b18 next=129c43a70 next-in-flow=129c43a70 {60,4560,432,750} vis-overflow=-30,0,924,750 scr-overflow=0,0,432,750 [state=02000048b1600000] [content=18a13a780] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][0,2,F] 
                    >
                    line 129c43b00: count=2 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x100] {60,5310,2160,750} vis-overflow=22,5310,2228,750 scr-overflow=60,5310,2160,750 <
                      Text(8)"world"@129c43a70 parent=129c32b18 next=129c33bf8 prev-in-flow=129c33ac0 {60,5310,2160,750} vis-overflow=-38,0,2228,750 scr-overflow=0,0,2160,750 [state=0200004890600004] [content=18a13a780] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][2,5,T] 
                      Placeholder(span)(9)@129c33bf8 parent=129c32b18 next=129c33c68 {2220,5880,0,0} [state=0000000000200000] [content=129bfca60] [sc=129c28730:-moz-oof-placeholder^0] outOfFlowFrame=Block(span)(9)@129c33b48
                    >
                    line 129c43bf0: count=1 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x100] {60,6060,432,750} vis-overflow=30,6060,924,750 scr-overflow=60,6060,432,750 <
                      Text(10)"!\n"@129c33c68 parent=129c32b18 {60,6060,432,750} vis-overflow=-30,0,924,750 scr-overflow=0,0,432,750 [state=02000048b0600000] [content=18a13a800] [sc=129c32d80:-moz-text] [run=129c2e300][run length=36][0,2,T] 
                    >
                    BulletList 0x129c352e8 <
                      Bullet(li)(3)@129c35240 parent=129c32b18 {-1296,60,1296,750} [state=0200004800000000] [content=129bfc790] [sc=129c33e00:-moz-list-number]
                    >
                  >
                >
```

Comment 19

[Jeremy Chen ([e-mail])]

Ok, now I'm more certain that fixing this during building the BuildTextRuns pipeline may not work. I traced the whole process of BuildTextRuns [1], and compared the break related intermediate info, all the break related info are exact the same between out_of_flow case and in-flow case. Here's the much reduced testcase I use for the test and verification.
 

```
<!DOCTYPE html>
<html><head>
<meta http-equiv="content-type" content="text/html; charset=UTF-8"><meta charset="utf-8">
<style>
body { font: 12px monospace; }
ol { width: 40ch; float: left; }
li { border: 1px solid silver; margin: 1em 4ch; }
.out_of_flow li span { position: absolute; }
</style>

</head><body><ol class="out_of_flow">
(A) With out-of-flow empty spans:<br>
<li style="width: 3ch">
hello<span></span>, cruel<span></span>, line<span></span>breaking<span></span>, world<span></span>!
</li>
</ol>

<ol>
(B) With in-flow empty spans:<br>
<li style="width: 3ch">
hello<span></span>, cruel<span></span>, line<span></span>breaking<span></span>, world<span></span>!
</li>
</ol>

</body></html>
```

The BuildTextRuns pipeline goes as follows:

1. Prescan each frame belongs to the current text run by iteratively calling BuildTextRunsScanner::ScanFrame [2] in BuildTextRuns.

2. After finishing scan all the frames, call BuildTextRunsScanner::FlushFrames [3] (which would call BuildTextRunsScanner::BuildTextRunForFrames [4]) to make/build the text run.

3. Once we finish creating the text run, we set break sinks via BuildTextRunsScanner::SetupBreakSinksForTextRun [5].

4. In BuildTextRunsScanner::SetupBreakSinksForTextRun, we call nsLineBreaker::AppendText [6] to feed each text chunk into the linebreaker for analysis.

5. Finally, we call SetBreaks [7] to set the potential linebreaks according to the analysis result, then finish the BuildTextRuns procedure.


In the all 5 steps, the break related intermediate data are all the same (between OOF case and in-flow case). I'm still unclear what later procedure could make the break opportunities different.... keep digging...



[1] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/generic/nsTextFrame.cpp#1445
[2] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/generic/nsTextFrame.cpp#1912
[3] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/generic/nsTextFrame.cpp#1666
[4] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/generic/nsTextFrame.cpp#2062
[5] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/generic/nsTextFrame.cpp#2577
[6] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/dom/base/nsLineBreaker.cpp#330
[7] https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/layout/generic/nsTextFrame.cpp#1112

Comment 20

[Jeremy Chen ([e-mail])]

It seems that the break opportunity is introduced when we call nsLineLayout::ReflowFrame for an OOF frame here [1]. The logic here is for recording a soft break opportunity after a frame that its content can't be part of a text run. However, as for an OOF frame, though it can't be part of an in-flow text run, it also can't/won't break the existing in-flow text run either. (see the investigation results in comment 18 and comment 19) So, we probably shouldn't set the line-break-after for all OOF frames here.


[1] https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/layout/generic/nsLineLayout.cpp#1118-1123

Comment 21

[Jeremy Chen ([e-mail])]

Attachment: Bug 1283222 - do not record soft break opportunities for out-of-flow frames.

We do want to record soft break opportunity after a frame that its content can't
be part of the existing in-flow text run, since this normally should be a text run
boundary. However, though an out-of-flow frame can't be part of the existing
in-flow text run, it also doesn't break it eigher. In fact, the texts after the
out-of-flow frames are able to continue the existing in-flow text run. So,
introducing a line-break-after opportunity in this case may cause an unexpected
line break for every out-of-flow frame.

In this patch, we add a condition to prevent us from recording soft break
opportunities for out-of-flow frames.

Review commit: https://reviewboard.mozilla.org/r/212358/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212358/

Comment 22

[Jeremy Chen ([e-mail])]

Comment on attachment 8942108 [details]
Bug 1283222 - do not record soft break opportunities for out-of-flow frames.

Hi Jonathan, according to the findings so far, it seems we should fix it while doing the line layout or some where nearby. There might be cases that I've missed, so ask for feedback for now.

Also, even if this patch is toward the right direction, it would be better to add more testcases before asking for review, such like cases for empty OOF frames, non-empty OOF frames, ...etc.

Comment 23

[Jonathan Kew [:jfkthame]]

Comment on attachment 8942108 [details]
Bug 1283222 - do not record soft break opportunities for out-of-flow frames.

This seems reasonable, on the basis that placeholder frames should have no effect on the layout (line-breaking or anything else) of the in-flow content.

Adding a set of testcases would definitely be good.

Comment 24

[Jeremy Chen ([e-mail])]

(In reply to Jonathan Kew (:jfkthame) from comment #23)
> Comment on attachment 8942108 [details]
> Bug 1283222 - do not record soft break opportunities for out-of-flow frames.
> 
> This seems reasonable, on the basis that placeholder frames should have no
> effect on the layout (line-breaking or anything else) of the in-flow content.
> 
> Adding a set of testcases would definitely be good.

Thanks for the feedback. I pushed a try run and fortunately this patch does not regress any existing test. :)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a71c5534fb172905d59591bc66d9ca693cc26cd

Comment 25

[Jeremy Chen ([e-mail])]

(In reply to Jonathan Kew (:jfkthame) from comment #13)
> Actually, checking [1], although "CSS does not fully define where soft wrap
> opportunities occur", it does explicitly mention that "out-of-flow elements
> do not introduce a forced line break or soft wrap opportunity in the flow".
> 
> This supports my position that in terms of the effect of the markup and
> properties here, IE11/Edge is correct; our behavior on column (A) is wrong,
> as is Safari's (in a similar but not-quite-identical way); and Chrome in
> addition shows buggy behavior in column (B).
> 
> So fixing this is required for CSS Text 3 conformance. And adding a relevant
> web-platform test, if one doesn't already exist (presumably not, or we'd be
> failing it!) would be good.
> 
> [1] https://drafts.csswg.org/css-text-3/#line-break-details

I'm trying to add tests into web-platform test, however, there are two folders, line-break and line-breaking, which confuses me a bit. Tests under both folders all point to css-text-3 specification and all relate to line-breaking behavior. Maybe we should merge them into one single folder?

Oh, wait, are tests under line-break folder specifically related to CSS line-break property, whereas other tests under line-breaking folder are related other line-breaking control issues? In that case, probably I should add tests into line-breaking folder.

Comment 26

[Jonathan Kew [:jfkthame]]

(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #25)
> Oh, wait, are tests under line-break folder specifically related to CSS
> line-break property, whereas other tests under line-breaking folder are
> related other line-breaking control issues? In that case, probably I should
> add tests into line-breaking folder.

Yes, that looks the most logical place, I think.

Comment 27

[Jeremy Chen ([e-mail])]

Attachment: Bug 1283222 - add WPT tests for empty out-of-flow elements.

According to CSS Text 3 Specification, out-of-flow elements do not introduce a
forced line break or soft wrap opportunity in the flow. So, empty out-of-flow
elements should be treated/rendered like empty in-flow elements. In this patch,
we add tests to ensure this.

Review commit: https://reviewboard.mozilla.org/r/212766/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212766/

Comment 28

[Jeremy Chen ([e-mail])]

Attachment: comparisons for non-empty OOF elements

This is a screenshot for replacing the empty OOF <span> element with a OOF <span> element that consists a single blue 'N' character. The results from left to right are Firefox (w/ the WIP applied), Edge, Safari, and Chrome. None of them are identical, which makes me a bit hesitate to add non-empty tests to WPT....

The tested html file is as follows:

```
<!doctype html>
<html>
<meta charset="utf-8">
<style>
body { font: 12px monospace; }
ol { width: 40ch; float: left; }
li { border: 1px solid silver; margin: 1em 4ch; }
.out_of_flow li span {
  position: absolute;
  color: blue;
}
</style>
<body>
<ol class="out_of_flow">
<li style="width: 3ch">
hello<span>N</span>, cruel<span></span>, line<span></span>breaking<span></span>, world<span></span>!
</li>
<li style="width: 4ch">
hello<span>N</span>, cruel<span></span>, line<span></span>breaking<span></span>, world<span></span>!
</li>
<li style="width: 5ch">
hello<span>N</span>, cruel<span></span>, line<span></span>breaking<span></span>, world<span></span>!
</li>
<li style="width: 6ch">
hello<span>N</span>, cruel<span></span>, line<span></span>breaking<span></span>, world<span></span>!
</li>
<li style="width: 12ch">
hello<span>N</span>, cruel<span></span>, line<span></span>breaking<span></span>, world<span></span>!
</li>
</ol>
</body>
</html>
```

Comment 29

[Jeremy Chen ([e-mail])]

As comment 28 said, I'm not sure if we should add tests to WPT for non-empty out-of-flow elements at the moment.

The empty out-of-flow elements seems more trivial to me, so I'm going to ask for review for the existing patchset.

Comment 30

[Jeremy Chen ([e-mail])]

try looks positive: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8ebe1030a8ca801126063c67fad6a9feb1da7b6

Comment 31

[Jeremy Chen ([e-mail])]

(In reply to Jeremy Chen [:jeremychen] UTC+8 from comment #29)
> As comment 28 said, I'm not sure if we should add tests to WPT for non-empty
> out-of-flow elements at the moment.
> 

I've dived into Bug 1418472 recently, and I think the issue there is more related to non-empty out-of-flow elements, so maybe we can add tests (whether for WPT or not) there.

Comment 32

[Jeremy Chen ([e-mail])]

Sorry, I may not be able to keep working on this bug.

Comment 33

[Jonathan Kew [:jfkthame]]

(In reply to Jeremy Chen [:jeremychen] UTC+8 (away for the moment) ([e-mail]) from comment #32)
> Sorry, I may not be able to keep working on this bug.

Sorry to hear that. Thanks for the investigations you've done. I'm hoping to get this reviewed and finished off soon anyhow, as your current patch here looks like a good step forward.

Comment 34

[Jonathan Kew [:jfkthame]]

Comment on attachment 8942514 [details]
Bug 1283222 - add WPT tests for empty out-of-flow elements.

Obsoleting this as I'm going to post an alternative patch with new WPT line-breaking tests to cover this bug.

Comment 35

[Jonathan Kew [:jfkthame]]

Comment on attachment 8942108 [details]
Bug 1283222 - do not record soft break opportunities for out-of-flow frames.

Obsoleting this attachment as I'm moving this patch to phabricator for review & landing.

Comment 36

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1283222 - Add some extra WPT line-breaking testcases. r?emilio

Comment 37

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1283222 - Do not record soft break opportunities for out-of-flow frames. r?emilio

We do want to record soft break opportunity after a frame whose content can't
be part of the existing in-flow text run, since this normally should be a text run
boundary. However, though an out-of-flow frame can't be part of the existing
in-flow text run, it also doesn't break it either. In fact, the texts after the
out-of-flow frames are able to continue the existing in-flow text run. So,
introducing a line-break-after opportunity in this case may cause an unexpected
line break for every out-of-flow frame.

In this patch, we add a condition to prevent us from recording soft break
opportunities for out-of-flow frames.

Depends on D28624

Comment 38

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #36)

Posted file Bug 1283222 - Add some extra WPT line-breaking testcases.

Just FTR, the line-breaking-015.html testcase here isn't really related to this Gecko bug, but I included it anyhow because it does fail in Blink's version of a similar bug, and AFAICS there's no current test coverage that reveals this. (Reported the Chrome failure as https://bugs.chromium.org/p/chromium/issues/detail?id=955496.)

Testcases 016 and 017 are this bug, and will be fixed by Jeremy's patch above.

Comment 39

[Jonathan Kew [:jfkthame]]

(In reply to Jonathan Kew (:jfkthame) from comment #38)

the line-breaking-015.html testcase here isn't really related to this Gecko bug

Turns out it actually fails on tryserver linux64, though, not because of unwanted line-breaking but because the presence of the <b> element may affect the normal line-height of the test div. Trying again with an explicit line-height:2 on that element...

Comment 40

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/093bf5cadbcc
Add some extra WPT line-breaking testcases. r=emilio
https://hg.mozilla.org/integration/autoland/rev/fb82d1aa97d0
Do not record soft break opportunities for out-of-flow frames. r=emilio

Comment 41

[Cosmin Sabou [:CosminS]]

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=busted&fromchange=fb82d1aa97d0c8ff9d0e853b4b21a14b5ba82245&tochange=a302746ead451eccadf5bb36553cd5dc8f1ce134&selectedJob=242291469

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=242291469&repo=autoland&lineNumber=30931

Backout link: https://hg.mozilla.org/integration/autoland/rev/a302746ead451eccadf5bb36553cd5dc8f1ce134

[task 2019-04-24T12:37:34.470Z] 12:37:34 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/angle/targets/translator'
[task 2019-04-24T12:37:34.491Z] 12:37:34 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/layout/generic'
[task 2019-04-24T12:37:34.495Z] 12:37:34 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/gcc/bin/g++ -o Unified_cpp_layout_generic2.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/obj-firefox/layout/generic -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/plugins/base -I/builds/worker/workspace/build/src/layout/base -I/builds/worker/workspace/build/src/layout/forms -I/builds/worker/workspace/build/src/layout/painting -I/builds/worker/workspace/build/src/layout/style -I/builds/worker/workspace/build/src/layout/svg -I/builds/worker/workspace/build/src/layout/tables -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/docshell/base -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/html -I/builds/worker/workspace/build/src/dom/xul -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/workspace/build/src/obj-firefox/dist/include/cairo -I/builds/worker/workspace/build/src/widget/gtk/compat-gtk3 -pthread -I/usr/include/gtk-3.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/gtk-3.0/unix-print -Wno-error=shadow -MD -MP -MF .deps/Unified_cpp_layout_generic2.o.pp /builds/worker/workspace/build/src/obj-firefox/layout/generic/Unified_cpp_layout_generic2.cpp
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/layout/generic/Unified_cpp_layout_generic2.cpp:128:0:
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - /builds/worker/workspace/build/src/layout/generic/nsLineLayout.cpp: In member function 'void nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, nsLineLayout::ReflowOutput*, bool&)':
[task 2019-04-24T12:37:34.496Z] 12:37:34 ERROR - /builds/worker/workspace/build/src/layout/generic/nsLineLayout.cpp:893:8: error: variable 'placedFloat' set but not used [-Werror=unused-but-set-variable]
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - bool placedFloat = false;
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - ^~~~~~~~~~~
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/layout/generic/Unified_cpp_layout_generic2.cpp:2:0:
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp: In static member function 'static int32_t nsFrame::GetLineNumber(nsIFrame*, bool, nsIFrame**)':
[task 2019-04-24T12:37:34.496Z] 12:37:34 WARNING - /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:8675:42: warning: 'thisBlock' may be used uninitialized in this function [-Wmaybe-uninitialized]
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - return it->FindLineContaining(thisBlock);
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - ^
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/nsSize.h:10:0,
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - from /builds/worker/workspace/build/src/layout/base/LayoutConstants.h:12,
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - from /builds/worker/workspace/build/src/layout/generic/nsIFrame.h:54,
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - from /builds/worker/workspace/build/src/layout/xul/nsBox.h:12,
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - from /builds/worker/workspace/build/src/layout/generic/nsFrame.h:15,
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - from /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:9,
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - from /builds/worker/workspace/build/src/obj-firefox/layout/generic/Unified_cpp_layout_generic2.cpp:2:
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCoord.h: In member function 'mozilla::LogicalSize nsFrame::ComputeSizeWithIntrinsicDimensions(gfxContext*, mozilla::WritingMode, const mozilla::IntrinsicSize&, nsSize, const mozilla::LogicalSize&, const mozilla::LogicalSize&, const mozilla::LogicalSize&, const mozilla::LogicalSize&, nsIFrame::ComputeSizeFlags)':
[task 2019-04-24T12:37:34.496Z] 12:37:34 WARNING - /builds/worker/workspace/build/src/obj-firefox/dist/include/nsCoord.h:70:11: warning: 'bSize' may be used uninitialized in this function [-Wmaybe-uninitialized]
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - return (int64_t(aMult1) * int64_t(aMult2) / int64_t(aDiv));
[task 2019-04-24T12:37:34.496Z] 12:37:34 INFO - ^~~~~~~~~~~~~~~
[task 2019-04-24T12:37:34.497Z] 12:37:34 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/layout/generic/Unified_cpp_layout_generic2.cpp:2:0:
[task 2019-04-24T12:37:34.497Z] 12:37:34 INFO - /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:5890:38: note: 'bSize' was declared here
[task 2019-04-24T12:37:34.497Z] 12:37:34 INFO - nscoord iSize, minISize, maxISize, bSize, minBSize, maxBSize;
[task 2019-04-24T12:37:34.497Z] 12:37:34 INFO - ^~~~~
[task 2019-04-24T12:37:34.497Z] 12:37:34 WARNING - /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:6089:45: warning: 'iSize' may be used uninitialized in this function [-Wmaybe-uninitialized]
[task 2019-04-24T12:37:34.497Z] 12:37:34 INFO - stretchI != eStretch && tentISize > iSize) {
[task 2019-04-24T12:37:34.497Z] 12:37:34 INFO - ^
[task 2019-04-24T12:37:34.497Z] 12:37:34 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/layout/generic/Unified_cpp_layout_generic2.cpp:47:0:
[task 2019-04-24T12:37:34.498Z] 12:37:34 INFO - /builds/worker/workspace/build/src/layout/generic/nsGfxScrollFrame.cpp: In member function 'void mozilla::ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder*, const nsDisplayListSet&)':
[task 2019-04-24T12:37:34.499Z] 12:37:34 WARNING - /builds/worker/workspace/build/src/layout/generic/nsGfxScrollFrame.cpp:3265:30: warning: 'extraContentBoxClipForNonCaretContent' may be used uninitialized in this function [-Wmaybe-uninitialized]
[task 2019-04-24T12:37:34.499Z] 12:37:34 INFO - cache);
[task 2019-04-24T12:37:34.499Z] 12:37:34 INFO - ^
[task 2019-04-24T12:37:34.499Z] 12:37:34 INFO - /builds/worker/workspace/build/src/layout/generic/nsGfxScrollFrame.cpp:3430:38: note: 'extraContentBoxClipForNonCaretContent' was declared here
[task 2019-04-24T12:37:34.499Z] 12:37:34 INFO - Maybe<const DisplayItemClipChain*> extraContentBoxClipForNonCaretContent;
[task 2019-04-24T12:37:34.499Z] 12:37:34 INFO - ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2019-04-24T12:37:34.499Z] 12:37:34 INFO - cc1plus: all warnings being treated as errors
[task 2019-04-24T12:37:34.499Z] 12:37:34 INFO - /builds/worker/workspace/build/src/config/rules.mk:805: recipe for target 'Unified_cpp_layout_generic2.o' failed

Comment 42

[Jonathan Kew [:jfkthame]]

Argh, sorry - will fix.

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e92c0bda3d9
Add some extra WPT line-breaking testcases. r=emilio
https://hg.mozilla.org/integration/autoland/rev/121fd81292cb
Do not record soft break opportunities for out-of-flow frames. r=emilio

Comment 44

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1e92c0bda3d9
https://hg.mozilla.org/mozilla-central/rev/121fd81292cb

Comment 45

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16528 for changes under testing/web-platform/tests

Comment 47

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged