I'm primarily chiming in here in hopes of getting something out in the wild to encourage other implementers.

Fingers crossed the renaming of -moz-hidden-unscrollable fits right into the definition of https://drafts.csswg.org/css-overflow-3/#valdef-overflow-clip and we can see if the heavily debated position:sticky issue https://github.com/w3c/csswg-drafts/issues/865 can be done away with.

Any moz devs able to describe what is still needed relating to the "dev-doc-needed" keyword here?

I think the dev-doc-needed is just meant to apply when we fix it, so mdn compatibility docs get updated. The work here is:

• Checking that the -moz-hidden-unscrollable matches the spec.
• Confirm with the CSSWG that the feature is ready to ship.
• Do the renaming in the code, and probably tests as well.

I can mentor the code part of it (not sure I'd have cycles to tackle it myself right now).

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

I think the dev-doc-needed is just meant to apply when we fix it, so mdn compatibility docs get updated.

Correct. And it would be a great help for the MDN team if the implementers could set it right when filing bugs for implementing web platform features.

Sebastian

I'd be interested in picking up this bug, if you all would be amiable to the idea. I've taken the initiative to familiarize myself with the CSS spec for overflow as well as attempted to somewhat expose myself to the implementation in the layout frame code.

Regarding Comment 1, based on what I'm seeing within the Gecko implementation of overflow, it appears that the position: sticky property would behave the same with MozHiddenUnscrollable as it does with auto/hidden. Is this a two-part bug in which we want to formally support overflow: clip as well as resolve the sticky issue? Or should that be addressed as a separate bug?

(In reply to Jeremy Ir from comment #4)

I'd be interested in picking up this bug, if you all would be amiable to the idea.

Yes, that'd be lovely!

Regarding Comment 1, based on what I'm seeing within the Gecko implementation of overflow, it appears that the position: sticky property would behave the same with MozHiddenUnscrollable as it does with auto/hidden. Is this a two-part bug in which we want to formally support overflow: clip as well as resolve the sticky issue? Or should that be addressed as a separate bug?

I don't think that's true, what makes you think that? In particular, Overflow::MozHiddenUnscrollable behaves as non-scrollable:

https://searchfox.org/mozilla-central/rev/b2015fdd464f598d645342614593d4ebda922d95/layout/style/nsStyleStruct.h#2126

Which means that it should not create a scroll frame, and thus position: sticky wouldn't be constrained by it. Though there may be some bugs around that... If so either separate bug or posting multiple patches to the bug should be acceptable.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

ate bug?

I don't think that's true, what makes you think that? In particular, Overflow::MozHiddenUnscrollable behaves as non-scrollable:

I modified the repro example from the github thread (https://codepen.io/valtlai/pen/XVrbPY) with overflow-x:-moz-hidden-unscrollable in place of overflow-x: auto and was seeing the same behavior where the table headers were not sticking to the top of the scroll frame (FF 66.0.4, 64-bit, Win7). From that, I extrapolated that the sticky bug was at least partially unrelated to the non-scrollability of the element.

I may be missing something blatant that is producing this behavior, so please let me know, as this would impact my ability to test any changes I make.

(In reply to Jeremy Ir from comment #6)

I modified the repro example from the github thread (https://codepen.io/valtlai/pen/XVrbPY) with overflow-x:-moz-hidden-unscrollable in place of overflow-x: auto and was seeing the same behavior where the table headers were not sticking to the top of the scroll frame (FF 66.0.4, 64-bit, Win7). From that, I extrapolated that the sticky bug was at least partially unrelated to the non-scrollability of the element.

I may be missing something blatant that is producing this behavior, so please let me know, as this would impact my ability to test any changes I make.

Yeah, you're getting fooled by this rule:

https://drafts.csswg.org/css-overflow/#overflow-properties

Computed Value: as specified, except with visible/clip computing to auto/hidden (respectively) if one of overflow-x or overflow-y is neither visible nor clip.

Which is implemented here:

https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/servo/components/style/style_adjuster.rs#434

(Ignore the "deprecated" bit, I guess, since it was ressurrected on specs and there are valid use-cases for it :-))

So you need overflow: clip, not just overflow-x: clip.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

So you need overflow: clip, not just overflow-x: clip.

Yep, that did the trick.

Would you mind helping me clarify something from the spec/code?

Computed Value: as specified, except with visible/clip computing to auto/hidden (respectively) if one of overflow-x or overflow-y is neither visible nor clip.

As I understand it, the initial/default value for overflow-y is visible. Based on the changes I used in Comment 6, the set value of overflow-x would be clip and the set value of overflow-y would visible, so the computed value should maintain the overflow-x: clip. Is my understanding of the spec itself correct?

The spec seems to allow for combinations of clip & visible. However the implementation in style_adjuster.rs seems to require both overflow directions to be set to visible for either direction to be set to visible (suggesting my understanding of the spec to be incorrect).

Hmm, I think your reading of the spec is correct. There's various code all around that make assumptions that changing that would break, e.g.:

https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/layout/generic/nsFrame.cpp#9197

Seems that wording was introduced in https://github.com/w3c/csswg-drafts/issues/1971, relatively recently.

It doesn't seem insanely impossible to handle, though there are a lot of places relying on that, see all the callers of:

https://searchfox.org/mozilla-central/rev/e7d9a8749303b39dadcc0e18ea0d60a570a68145/layout/generic/nsFrame.h#588

Extend3DContext is the one where I'm not quite sure what's supposed to happen. Also, conceptually, clipping happens in rects. I assume you can define overflow: clip visible as clipping on a rectangle with definite bounds in one axis, and infinite bounds in the other.

Matt, David or Mats (cc'd) are probably much more familiar with overflow during painting and computation, so can probably have a better judgement on whether visible and clip combinations are reasonable to support.

I think that sounds feasible.

Clipping does happen to a rect, but it seems like we could use the frame's pre-transform overflow area as the source of the axis for ones that are unclipped.

It looks like our computation of the overflow rect includes any necessary clipping, so using that as the clip (to enforce that the rendered overflow matches what was computed) should do the right thing.

For Extend3DContext, it looks like the latest draft (css-transforms-2 ED, transform-style:preserve-3d) suggests that 'clip' shouldn't stop the function from returning true (it just mentions overflow, not the per-axis versions). I don't think that's correct though, any form of clipping is super hard to implement in 3d space. I think any value that is not 'visible' (for either axis) should make Extend3DContext return false. I'm working on some rewrites for 3d transforms, will fix this.

Tab mentions both uses cases which indicates to me that they'll add what we currently do for -moz-hidden-unscrollable as a separate new value eventually. So I'm updating the title of this bug accordingly. To be clear: we should leave all code related to -moz-hidden-unscrollable as is and write new code for clip.

(In reply to Mats Palmgren (:mats) from comment #11)

Tab mentions both uses cases which indicates to me that they'll add what we currently do for -moz-hidden-unscrollable as a separate new value eventually. So I'm updating the title of this bug accordingly. To be clear: we should leave all code related to -moz-hidden-unscrollable as is and write new code for clip.

Pretty sure -moz-hidden-unscrollable doesn't create a scroll container and behaves like clip is supposed to behave. Tab wants yet another thing.

Oh, my mistake. I thought we created a scroll frame but suppressed scrolling for that thing.
OK, then we should definitely just rename it and make it deal with potentially having visible in one axis.

Alright, I'll implement this in 2 diffs, first with the name change, then with the functionality change.

Also, would someone mind changing the Assignee field to me please? I seem to not have permissions to do so.

Renamed all instances of -moz-hidden-unscrollable with clip for the overflow
CSS property, as well as the related enumeration.

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jir.opensource, could you have a look please?
For more information, please visit auto_nag documentation.

The patch should have nothing to do with this bug. No code has been submitted from this and it's not a land-blocking bug.

I may have to forgo fixing the outstanding bug for this item. There are a lot of places that ShouldApplyOverflowClipping touches and I've unfortunately struggled to get a grasp of how all of the calculations function.

Emilio, what's the status of this patch ?

Per spec overflow: clip should work in only one of the two axes, as long as the other is visible, so we need to make that work before landing the patch.

I'll take a stab at finishing this since I have an idea of how to implement the layout part correctly...

Depends on D73716

So if I understand the spec[1] correctly, the overflow value on <body> is supposed to be propagated to the viewport in this case. That is, you should see a large blue rectangle that is clipped by the viewport on the right/bottom side (with no scrollbars, since it should behave as hidden). It should not be clipped by <body> (like we currently do) because the spec says:

The element from which the value is propagated must then have a used overflow value of visible.

[1]
https://drafts.csswg.org/css-overflow/#overflow-propagation

Hmm, that is a bit unfortunate implementation-wise :(

Indeed, I suspect we need a global frame bit for this. Anyway, I filed bug 1635473 to fix that separately first.

I think it's worth asking whether that's intentional -- another reasonable option is that only the scrollbar-related ones propagate to the viewport.

What do other implementations do?

Chrome and Safari don't implement clip (nor some -webkit- version of it) AFAICT.

I don't feel strongly either way, but the spec behavior was resolved somewhat recently by the WG:
https://github.com/w3c/csswg-drafts/issues/1971#issuecomment-380148099

I think that resolution was about something slightly different: it was already assuming that it does propagate, and answering the question of what happens when the value gets to the viewport (which must have a scrolling mechanism and possibly scrollbars).

Resolving what happens after it has propagated is effectively a resolution that it should propagate IMO. Otherwise those discussions would be rather pointless, or at least would have come to the opposite conclusion that it shouldn't propagate in the first place. So the resolution being what it is gives me the impression that the WG's consensus is that clip does propagate and then also decides what happens when it does.

Feel free to raise a spec issue though if you think it's worth revisiting.

I think you're reading an awful lot into a discussion that took about 30 seconds (this line and the two following) and involved 2 people. Probably most of the rest of the room hadn't understood what issue was being discussed.

The propagation was also discussed in some detail by Florian and Tab in the issue before that, so it still appears like a deliberate decision to me.

Document::IsPotentiallyScrollable (which I'm going to make a change to in bug 1654099) will need to be updated to check for clip values.

(In reply to Cameron McCormack (:heycam) from comment #33)

Document::IsPotentiallyScrollable (which I'm going to make a change to in bug 1654099) will need to be updated to check for clip values.

It looks like you've already changed it to using IsScrollableOverflow() instead of checking individual style values, so I think we're fine.

Backed out for reftest failures.

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

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=1e7b32808be8fdce40874dcdf417beecedbdc644&searchStr=Windows%2C10%2Cx64%2CWebRender%2Cdebug%2CReftests%2Ctest-windows10-64-qr%2Fdebug-reftest-e10s%2CR4&selectedTaskRun=Ky1JKXllSICM7nQhN_P2uA.0

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

Those failures appears to be small differences in text anti-aliasing in some XUL tests.
I don't think this indicates a bug so I'll just add a fuzz factor for these tests in the reftest.list manifest...

https://hg.mozilla.org/mozilla-central/rev/cf1609de8842
https://hg.mozilla.org/mozilla-central/rev/e0fcac18ed9a

I've updated the MDN page https://wiki.developer.mozilla.org/en-US/docs/Web/CSS/overflow

BCD has been merged so will show up eventually.