Bug 93809 – [CSS Regions] Setting region-break-after: always on a <br> element does not cause a region break

• Component: CSS
• Status: RESOLVED WONTFIX

[Mihai Balan]

Setting region-break-before (or region-break-after) to always on a <br> element does not cause a break, no matter the value used for the display property.

[Mihai Balan]

Created attachment 157946 [details]
Ref test highlighting the problem

[Mihai Balan]

As per the fragmentation spec and further discussions on www-style and IRC/whatwg, the code here should actually create a break.

[Manuel Rego Casasnovas]

BTW, I've started to take a look to this issue. I've tested it changing <br> by <span> and it works properly.

For the moment, I've been debugging the code and what I get is that the RenderBlock corresponding to the paragraph has no children while in the case of <span> it has children. I'll look deeper trying to understand why this RenderBlock is created without any children.

[Manuel Rego Casasnovas]

(In reply to comment #3)
> For the moment, I've been debugging the code and what I get is that the RenderBlock corresponding to the paragraph has no children while in the case of <span> it has children. I'll look deeper trying to understand why this RenderBlock is created without any children.

I was wrong in my assumption, checking the code that creates the RenderObjects it actually creates the paragraph and its children.

Them problem is that for the paragraph childrenInline() is TRUE (in the case of <br> and FALSE in the case of <span>) this causes that RenderBlock::layoutBlockChildren() is not called. You can see the if in RenderBlock::layoutBlock():

    if (childrenInline())
        layoutInlineChildren(relayoutChildren, repaintLogicalTop, repaintLogicalBottom);
    else
        layoutBlockChildren(relayoutChildren, maxFloatLogicalBottom);

The next step will be to understand why childrenInline() is TRUE in this situation.

[Michelangelo De Simone]

Still repros on today's nightly (r151543)

[Mihai Maerean]

Before anything else, fragmentation css style ([region|page|columns]-break) properties need to have bits that propagate to the containers to indicate the break situation between blocks.

[Manuel Rego Casasnovas]

(In reply to comment #4)
> Them problem is that for the paragraph childrenInline() is TRUE (in the case of <br> and FALSE in the case of <span>) this causes that RenderBlock::layoutBlockChildren() is not called. You can see the if in RenderBlock::layoutBlock():
> 
>     if (childrenInline())
>         layoutInlineChildren(relayoutChildren, repaintLogicalTop, repaintLogicalBottom);
>     else
>         layoutBlockChildren(relayoutChildren, maxFloatLogicalBottom);
> 
> The next step will be to understand why childrenInline() is TRUE in this situation.

childrenInline() is TRUE for <br> and FALSE for <span> because in the case of <span> RenderObject::isInline() returns FALSE, which causes that RenderBlock::makeChildrenNonInline() is called inside RenderBlock::addChildIgnoringAnonymousColumnBlocks(). However, <br> RenderObject::isInline() returns TRUE.

So all the children of <p> (the text nodes and the <br> nodes) have RenderObject::isInline() TRUE, so it's normal than childrenInline() is TRUE.

Maybe in this case <br> elements with a region break should be marked as non-inline or something like that.

[Manuel Rego Casasnovas]

Created attachment 205530 [details]
Example test fixing the issue adding an empty content

(In reply to comment #7)
> childrenInline() is TRUE for <br> and FALSE for <span> because in the case of <span> RenderObject::isInline() returns FALSE, which causes that RenderBlock::makeChildrenNonInline() is called inside RenderBlock::addChildIgnoringAnonymousColumnBlocks(). However, <br> RenderObject::isInline() returns TRUE.

Actually the reason why isInline() returns FALSE for <span> elements is because of they have "display:block", if you remove that line then the breaks don't work either with <span> tags.

It seems that the breaks only work for blocks and not for inline elements.

> Maybe in this case <br> elements with a region break should be marked as non-inline or something like that.

On the other hand in order to fix the issue with <br> elements is enough with adding "content: "";" in the CSS (as in the attached example). Because of before/after pseudo-elements are not applied if they don't have content. This is not needed for <span> elements because of they always have a content (at least an empty string).

[Mihnea Ovidenie]

(In reply to comment #8)
> Created an attachment (id=205530) [details]
> Example test fixing the issue adding an empty content
> 
> (In reply to comment #7)
> > childrenInline() is TRUE for <br> and FALSE for <span> because in the case of <span> RenderObject::isInline() returns FALSE, which causes that RenderBlock::makeChildrenNonInline() is called inside RenderBlock::addChildIgnoringAnonymousColumnBlocks(). However, <br> RenderObject::isInline() returns TRUE.
> 
> Actually the reason why isInline() returns FALSE for <span> elements is because of they have "display:block", if you remove that line then the breaks don't work either with <span> tags.
> 
> It seems that the breaks only work for blocks and not for inline elements.
> 

Yes, this is the definition of forced breaks: http://www.w3.org/TR/2012/WD-css3-break-20120823/#break-properties

> > Maybe in this case <br> elements with a region break should be marked as non-inline or something like that.
> 
> On the other hand in order to fix the issue with <br> elements is enough with adding "content: "";" in the CSS (as in the attached example). Because of before/after pseudo-elements are not applied if they don't have content. This is not needed for <span> elements because of they always have a content (at least an empty string).

I am confused about your mention of pseudo-elements. I see the fix with the content but i would rather see what it takes to fix it in the code. Probably such a fix would also fix for multicolumn element in this case.

[Manuel Rego Casasnovas]

(In reply to comment #9)
> (In reply to comment #8)
> > It seems that the breaks only work for blocks and not for inline elements.
> > 
> 
> Yes, this is the definition of forced breaks: http://www.w3.org/TR/2012/WD-css3-break-20120823/#break-properties

Ok, thanks for the link.

> > On the other hand in order to fix the issue with <br> elements is enough with adding "content: "";" in the CSS (as in the attached example). Because of before/after pseudo-elements are not applied if they don't have content. This is not needed for <span> elements because of they always have a content (at least an empty string).
> 
> I am confused about your mention of pseudo-elements. I see the fix with the content but i would rather see what it takes to fix it in the code. Probably such a fix would also fix for multicolumn element in this case.

Sorry for the comment about the pseudo-elements I read some information somewhere but I messed it up in my head.

Anyway, I've been taking a look to the code related with content, and why the <br> is considered a block when content is added. The reason is in the following method where style->hasContent() is checked:

RenderObject* HTMLBRElement::createRenderer(RenderArena* arena, RenderStyle* style)
{
     if (style->hasContent())
        return RenderObject::createObject(this, style);

     return new (arena) RenderBR(this);
}

If it hasn't content, it always create a RenderBR which is a RenderText, which is always inline, so breaks won't work in <br> elements without content.

In order to change it we could add a new condition in this if. I'll provide an initial patch doing it to see if it's or not a valid approach.

[Manuel Rego Casasnovas]

Created attachment 206330 [details]
Patch

This is an initial idea to fix the issue. If it's right I'll primprove the test case adding a link to the bug and some descripton and also provide a new baseline for some tests that are failing with this patch (fast/inline/inline-child-height-width-calc-crash.html and tables/mozilla/bugs/bug275625.html).

[Build Bot]

Comment on attachment 206330 [details]
Patch

Attachment 206330 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1050112

New failing tests:
fast/inline/inline-child-height-width-calc-crash.html
fullscreen/full-screen-iframe-with-max-width-height.html
tables/mozilla/bugs/bug275625.html

[Build Bot]

Created attachment 206354 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 206330 [details]
Patch

Attachment 206330 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/881741

New failing tests:
fast/inline/inline-child-height-width-calc-crash.html
tables/mozilla/bugs/bug275625.html

[Build Bot]

Created attachment 206357 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Build Bot]

Comment on attachment 206330 [details]
Patch

Attachment 206330 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/970497

New failing tests:
fast/inline/inline-child-height-width-calc-crash.html
tables/mozilla/bugs/bug275625.html

[Build Bot]

Created attachment 206567 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3

[Alexandru Chiculita]

(In reply to comment #11)
> Created an attachment (id=206330) [details]
> Patch
> 
> This is an initial idea to fix the issue. If it's right I'll primprove the test case adding a link to the bug and some descripton and also provide a new baseline for some tests that are failing with this patch (fast/inline/inline-child-height-width-calc-crash.html and tables/mozilla/bugs/bug275625.html).

I wouldn't change the way BR tags work when display: block is set. It seems like other browsers ignore that property on BR tags, too.

http://jsfiddle.net/PR9wb/1/

Might be worth looking at how RenderBlock::layoutRunsAndFloats treats inline BR tags that have "clear" set.

[Brady Eidson]

Comment on attachment 206330 [details]
Patch

Assuming that patches for review since 2013 are stale, r-

[Brent Fulgham]

CSS Regions were removed in Bug 174978.