Introduce isScrollContainer helper and start using it.
Created attachment 433393 [details] Patch
Created attachment 433396 [details] Patch
Comment on attachment 433396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433396&action=review > Source/WebCore/ChangeLog:14 > + The overflow property decides whether the RenderObject > + is scrollable, isScrollContainer captures this. > + Right now isScrollContainer will only return false for > + overflow visible, but the upcoming overflow clip is > + also not scrollable. Let's choose a good name for this. isScrollContainer() is vague because the element might not be scrollable, and overflow:hidden is not user-scrollable, but is programmatically scrollable. I want "overflow" in the name, because it's about the overflow property. We also need to decide if this helper will return true or false for overflow:clip, and if we need another helper for the visible/clip combination. For the current behavior, I suggest: hasNonVisibleOverflow() For the future "potentially scrollable" case I suggest: hasPotentiallyScrollableOverfow() or something.
Created attachment 433435 [details] Patch
Comment on attachment 433396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433396&action=review >> Source/WebCore/ChangeLog:14 >> + also not scrollable. > > Let's choose a good name for this. > > isScrollContainer() is vague because the element might not be scrollable, and overflow:hidden is not user-scrollable, but is programmatically scrollable. I want "overflow" in the name, because it's about the overflow property. > > We also need to decide if this helper will return true or false for overflow:clip, and if we need another helper for the visible/clip combination. > > For the current behavior, I suggest: > > hasNonVisibleOverflow() > > For the future "potentially scrollable" case I suggest: > > hasPotentiallyScrollableOverfow() or something. That makes sense. But hasNonVisibleOverflow would just be a rename of hasOverflowClip I think (chromium did this)? I think it helps readibility and overflow: clip to add a method hasProgrammaticallyScrollableOverflow for programmatic scroll API, let me know what you think.
(In reply to Rob Buis from comment #5) > Comment on attachment 433396 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=433396&action=review > > >> Source/WebCore/ChangeLog:14 > >> + also not scrollable. > > > > Let's choose a good name for this. > > > > isScrollContainer() is vague because the element might not be scrollable, and overflow:hidden is not user-scrollable, but is programmatically scrollable. I want "overflow" in the name, because it's about the overflow property. > > > > We also need to decide if this helper will return true or false for overflow:clip, and if we need another helper for the visible/clip combination. > > > > For the current behavior, I suggest: > > > > hasNonVisibleOverflow() > > > > For the future "potentially scrollable" case I suggest: > > > > hasPotentiallyScrollableOverfow() or something. > > That makes sense. But hasNonVisibleOverflow would just be a rename of > hasOverflowClip I think (chromium did this)? That's the point! > I think it helps readibility and overflow: clip to add a method > hasProgrammaticallyScrollableOverflow for programmatic scroll API, let me > know what you think. But hasProgrammaticallyScrollableOverflow is also ambiguous: is it hasProgrammaticallyOrUserScrollableOverflow ?
Created attachment 433446 [details] Patch
Comment on attachment 433396 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433396&action=review >>>> Source/WebCore/ChangeLog:14 >>>> + also not scrollable. >>> >>> Let's choose a good name for this. >>> >>> isScrollContainer() is vague because the element might not be scrollable, and overflow:hidden is not user-scrollable, but is programmatically scrollable. I want "overflow" in the name, because it's about the overflow property. >>> >>> We also need to decide if this helper will return true or false for overflow:clip, and if we need another helper for the visible/clip combination. >>> >>> For the current behavior, I suggest: >>> >>> hasNonVisibleOverflow() >>> >>> For the future "potentially scrollable" case I suggest: >>> >>> hasPotentiallyScrollableOverfow() or something. >> >> That makes sense. But hasNonVisibleOverflow would just be a rename of hasOverflowClip I think (chromium did this)? >> >> I think it helps readibility and overflow: clip to add a method hasProgrammaticallyScrollableOverflow for programmatic scroll API, let me know what you think. > > That's the point! Oops, I thought somehow the bug was about adding a new method. Done.
(In reply to Simon Fraser (smfr) from comment #6) > > I think it helps readibility and overflow: clip to add a method > > hasProgrammaticallyScrollableOverflow for programmatic scroll API, let me > > know what you think. > > But hasProgrammaticallyScrollableOverflow is also ambiguous: is it > hasProgrammaticallyOrUserScrollableOverflow ? My idea was to restrict that one to programmatic scrolling only. I am not sure yet if we need a hasUserScrollableOverflow. Both may be simple aliases though, anyway this is probably pretty minor, will have another look at that kind of naming tomorrow.
Committed r279918 (239667@main): <https://commits.webkit.org/239667@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433446 [details].
<rdar://problem/80594572>