Bug 248432 – Remove non-standard: overflow: -webkit-paged-* (x|y)

• Component: Layout and Rendering
• Status: NEW

[Ahmad Saleem]

Hi Team,

While going through Blink's commit, I came across another interesting one, where Safari is still failing test case (STP158 & Safari 16.1) compared to Chrome Canary 110 and Firefox Nightly 109:

Failing Test Case - https://jsfiddle.net/bajc7h9p/ (Can't scroll from yellow div)

Blink Commit - https://src.chromium.org/viewvc/blink?view=revision&revision=190656

Webkit Merge is difficult to 1-1 (At least for me), because this commit removed code, where I could've added this bit:

https://github.com/WebKit/WebKit/commit/38512761f8da553f42c566ea425d045351531469

Just wanted to raise bug for future purposes.

Thanks!

[Karl Dubost]

Instead it should probably be removed. 
https://bugs.chromium.org/p/chromium/issues/detail?id=940652

With 
```
data:text/html,<div%20style="line-height:2em;%20height:9em;%20overflow:-webkit-paged-y;%20background:yellow;">%20%20%20%20%20scroll%20me<br>%20%20%20%20%20Please%20scroll%20me<br>%20%20%20%20%20Pretty%20please%20scroll%20me<br>%20%20%20%20%20Pretty%20please%20scroll%20me!<br>%20%20%20%20%20This%20is%20where%20page%202%20starts<br>%20%20%20%20%20more%20page%202%20content<br>%20%20%20%20%20even%20more%20page%202%20content<br>%20%20%20%20%20PASS%20</div>
```

and `window.getComputedStyle(document.querySelector('div')).overflow`

Safari: `auto -webkit-paged-y`
Firefox: `visible`
Chrome: `visible`


https://searchfox.org/wubkat/search?q=WebkitPaged&path=&case=false&regexp=false

[Radar WebKit Bug Importer]

<rdar://problem/103000377>

[Ahmad Saleem]

(In reply to Karl Dubost from comment #1)
> Instead it should probably be removed. 
> https://bugs.chromium.org/p/chromium/issues/detail?id=940652
> 
> With 
> ```
> data:text/html,<div%20style="line-height:2em;%20height:9em;%20overflow:-
> webkit-paged-y;%20background:yellow;
> ">%20%20%20%20%20scroll%20me<br>%20%20%20%20%20Please%20scroll%20me<br>%20%20
> %20%20%20Pretty%20please%20scroll%20me<br>%20%20%20%20%20Pretty%20please%20sc
> roll%20me!
> <br>%20%20%20%20%20This%20is%20where%20page%202%20starts<br>%20%20%20%20%20mo
> re%20page%202%20content<br>%20%20%20%20%20even%20more%20page%202%20content<br
> >%20%20%20%20%20PASS%20</div>
> ```
> 
> and `window.getComputedStyle(document.querySelector('div')).overflow`
> 
> Safari: `auto -webkit-paged-y`
> Firefox: `visible`
> Chrome: `visible`
> 
> 
> https://searchfox.org/wubkat/
> search?q=WebkitPaged&path=&case=false&regexp=false

Sweet! If we can remove it then it is better since then we would more web-compatible.

[Myles C. Maxfield]

Why do we think removing this is safe? We generally don't remove things unless we are confident it's A) not used and B) causing us harm somehow

[Tim Nguyen (:ntim)]

(In reply to Myles C. Maxfield from comment #4)
> Why do we think removing this is safe? We generally don't remove things
> unless we are confident it's A) not used and B) causing us harm somehow

I don't know about A, but regarding B, this can cause behavior differences on websites between Safari and other browsers which is undesirable.

[Ahmad Saleem]

Blink - Intent To Remove - https://groups.google.com/a/chromium.org/g/blink-dev/c/_UhJEz04sqA

__________

https://chromestatus.com/metrics/feature/timeline/popularity/1867

^ Usage is about 0.005%

_______

Firefox -> Never supported this.

Blink / Chrome -> Removed it in 2019.

_______

So Safari is only browser with support for this.

[Karl Dubost]

The question becomes are there other WebKit-powered applications using this specific feature?

[Ahmad Saleem]

Local Page:

>> Source/WebCore/rendering/style/RenderStyleConstants.h

Line 273 & 274: enum class Overflow : uint8_t

Remove 'PagedX' and 'PagedY'

>> Source/WebCore/style/StyleAdjuster.cpp:

Remove following FIXME:

// FIXME: Once we implement pagination controls, overflow-x should default to hidden
// if overflow-y is set to -webkit-paged-x or -webkit-page-y. For now, we'll let it
// default to auto so we can at least scroll through the pages.

and following:

// Call setStylesForPaginationMode() if a pagination mode is set for any non-root elements. If these
    // styles are specified on a root element, then they will be incorporated in
    // Style::createForm_document.
    if ((style.overflowY() == Overflow::PagedX || style.overflowY() == Overflow::PagedY) && !(m_element && (m_element->hasTagName(htmlTag) || m_element->hasTagName(bodyTag))))
        style.setColumnStylesFromPaginationMode(WebCore::paginationModeForRenderStyle(style));

>> Source/WebCore/rendering/style/RenderStyleConstants.cpp:

In 'TextStream& operator<<(TextStream& ts, Overflow overflow)'

Remove case Overflow::PagedX: and case Overflow::PagedY:

>> Source/WebCore/rendering/RenderBlockFlow.cpp

In 'RenderBlockFlow::willCreateColumns', delete following:

// If overflow-y is set to paged-x or paged-y on the body or html element, we'll handle the paginating in the RenderView instead.
    if ((style().overflowY() == Overflow::PagedX || style().overflowY() == Overflow::PagedY) && !(isDocumentElementRenderer() || isBody()))
        return true;

>> Source/WebCore/page/LocalFrameView.h:

Delete 'Pagination::Mode paginationModeForRenderStyle'

>> Source/WebCore/page/LocalFrameView.cpp:

Delete 'Pagination::Mode paginationModeForRenderStyle'

and following comment:

'Don't set it at all. Values of Overflow::PagedX and Overflow::PagedY are handled by applyPaginationToViewPort().'


and in LocalFrameView::applyPaginationToViewport(), change it to this (* Not sure on this one *):

void LocalFrameView::applyPaginationToViewport()
{
    auto* document = m_frame->document();
    auto* documentElement = document ? document->documentElement() : nullptr;
    if (!documentElement || !documentElement->renderer()) {
        setPagination(Pagination());
        return;
    }
    Pagination pagination;
    setPagination(pagination);
}

>> Source/WebCore/css/parser/CSSPropertyParser.cpp:

In 'CSSPropertyParser::consumeOverflowShorthand': Delete following:

// FIXME: -webkit-paged-x or -webkit-paged-y only apply to overflow-y. If this value has been
        // set using the shorthand, then for now overflow-x will default to auto, but once we implement
        // pagination controls, it should default to hidden. If the overflow-y value is anything but
        // paged-x or paged-y, then overflow-x and overflow-y should have the same value.
        if (xValueID == CSSValueWebkitPagedX || xValueID == CSSValueWebkitPagedY)
            xValueID = CSSValueAuto;

>> Source/WebCore/css/CSSValueKeywords.in:

Delete following:

// overflow
-webkit-paged-x
-webkit-paged-y

>> Source/WebCore/css/CSSProperties.json:

Delete following:


},
                {
                    "value": "-webkit-paged-x",
                    "status": "non-standard"
                },
                {
                    "value": "-webkit-paged-y",
                    "status": "non-standard"

>> Source/WebCore/css/CSSPrimitiveValueMappings.h:

In 'constexpr CSSValueID toCSSValueID(Overflow e)': Delete following cases: 'case Overflow::PagedX:' and 'case Overflow::PagedY:'


In 'template<> constexpr Overflow fromCSSValueID(CSSValueID valueID)': Delete following cases: 'case CSSValueWebkitPagedX:' and 'case CSSValueWebkitPagedY:'

_____________

Just wanted to document for someone else to try locally.

[Ahmad Saleem]

@Karl - do you have any insight from internal Apple products perspective, which might rely on it?

[Karl Dubost]

I'm not sure, but it could probably be removed after the poking I have done internally. And given it has removed for a long time from chrome and was never be in Firefox. 

Content is probably not relying on it. 

Though on the other hand we can see things like

https://github.com/jarek-foksa/xel/blob/f977219f127492ab714e4b1c55c9726b6103aa29/themes/base.css#L792-L795


And comment like this
https://stackoverflow.com/questions/59599461/equivalent-to-uiwebviews-paginationmode-on-wkwebview


It is probably necessary to check if these cases have been solved. 


> Practically speaking these aren't used. Most developers use them accidentally, and typically when they are they force a new formatting context similar to setting overflow: hidden.
— https://developer.chrome.com/blog/chrome-75-deps-rems/