Closed Bug 914891 Opened 6 years ago Closed 6 years ago

"ASSERTION: Need a scrolling container" and crash (with position:sticky and fixed root)

Categories

(Core :: Layout: Positioned, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: jruderman, Assigned: coyotebush)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, crash, testcase)

Attachments

(3 files, 1 obsolete file)

With:
  user_pref("layout.css.sticky.enabled", true);

###!!! ASSERTION: Need a scrolling container: 'scrollFrame', file layout/generic/StickyScrollContainer.cpp, line 50

Crash [@ mozilla::StickyScrollContainer::StickyScrollContainerForFrame]
Attached file stack
Ah, we thought we might find some situation without a scroll container. That one makes sense.

Probably I should just bail more gently on anything sticky-related if there isn't a scroll container?
That would be easy enough.
Assignee: nobody → cford
Although this particular case will be a bit more involved; it's been nice to assume that StickyScrollContainerForFrame was infallible.
Huh, looks like that test case *doesn't* crash as a reftest...
(The nsComputedDOMStyle.cpp code for sticky already handles this okay.)
Attachment #803293 - Flags: review?(dholbert)
Comment on attachment 803293 [details] [diff] [review]
Bail gracefully on sticky positioning with no scroll container.

>diff --git a/layout/generic/StickyScrollContainer.h b/layout/generic/StickyScrollContainer.h
>--- a/layout/generic/StickyScrollContainer.h
>+++ b/layout/generic/StickyScrollContainer.h
>@@ -24,17 +24,17 @@ namespace mozilla {
> 
> class StickyScrollContainer MOZ_FINAL : public nsIScrollPositionListener
> {
> public:
>   /**
>    * Find the StickyScrollContainer associated with the scroll container of
>    * the given frame, creating it if necessary.
>    */
>-  static StickyScrollContainer* StickyScrollContainerForFrame(nsIFrame* aFrame);
>+  static StickyScrollContainer* GetStickyScrollContainerForFrame(nsIFrame* aFrame);


Update the documentation here, to indicate that this can return null & why.

r=me with that.
Attachment #803293 - Flags: review?(dholbert) → review+
Attachment #803293 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/82d94735ba7f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.