Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

break-before breaks layouting #58

Open
RonaldTreur opened this issue Dec 7, 2020 · 0 comments
Open

break-before breaks layouting #58

RonaldTreur opened this issue Dec 7, 2020 · 0 comments

Comments

@RonaldTreur
Copy link
Contributor

What goes wrong

Having an element with a CSS property break-before set to region, always or all breaks the polyfill in most cases. What happens is that extractOverflowingContent will correctly detect the element that has this property, upon which it decides to make the cut right before it. However, it only does this if it is not the "first" element (in the region). However, the current check only picks out very specific use-cases, and misses most.

The end result is that all regions are empty, except the last, because the algorithm could not move the content further and put it all in there.

The cause

The moment a single region can not contain all of the content (for whatever reason) extractOverflowingContent is being invoked. Once a suitable cut-off point has been detected, it will then traverse the content in the region bottom-up in order to detect any elements which have a break-before property set (or others, but that is irrelevant here).

It does so by executing the following code:

var current = r.endContainer; if(current.hasChildNodes()) { if(r.endOffset>0) { current=current.childNodes[r.endOffset-1] } };
var first = r.endContainer.firstChild;
do {
if(current.style) {
if(current != first) {
if(/(region|all|always)/i.test(cssCascade.getSpecifiedStyle(current,'break-before',undefined,true).toCSSString())) {
r.setStartBefore(current);
r.setEndBefore(current);
dontOptimize=true; // no algo involved in breaking, after all
}
}
if(current !== region) {
if(/(region|all|always)/i.test(cssCascade.getSpecifiedStyle(current,'break-after',undefined,true).toCSSString())) {
r.setStartAfter(current);
r.setEndAfter(current);
dontOptimize=true; // no algo involved in breaking, after all
}
}
}
} while(current = cssRegionsHelpers.getAllLevelPreviousSibling(current, region));

Its attempt to not cause this issue can be found on line 543, where it is trying to make sure this is not the first item:

if(current != first) {

Determining the first item is done on line 539:

var first = r.endContainer.firstChild;

However, this is not correct. The range's endContainer is (unless we are in a single region-spanning element) very likely not the element with which this region's content starts.

Possible fix

Interesting enough, a variation of this check is performed earlier in the code, in the layoutContentInNextRegionsWhenReady-method. Here the following code is used:

var first = region.firstElementChild;
var last = region.lastElementChild;
var current = first;
while(current) {
if(current != first) {
if(/(region|all|always)/i.test(cssCascade.getSpecifiedStyle(current,'break-before',undefined,true).toCSSString())) {
shouldSegmentContent = true; break;
}
}

This value for the first element (seen on line 130) is indeed the first element of the region and should most likely also be used by the extractOverflowingContent-method.

RonaldTreur added a commit to RonaldTreur/css-regions-polyfill that referenced this issue Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant