Closed Bug 1353529 Opened 3 years ago Closed 3 years ago

Crash when using IntersectionObserver in XUL pages

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: tschneider, Assigned: tschneider)

References

Details

Attachments

(1 file)

Using the IntersectionObserver API in XUL pages (e.g. in Add-Ons) causes crashes when trying to get to the root scrolling element.

STR:

Install the "Brief" feed reading extension from the Add-on marketplace.
Open the Mozilla Blog feed.
As of today, clicking on the feed entry with the title "A Public-Private Partnership for Gigabit Innovation and Internet Health" reproducibly causes a crash.

Crash reports:

https://crash-stats.mozilla.com/signature/?version=55.0a1&signature=mozilla%3A%3Adom%3A%3ADOMIntersectionObserver%3A%3AUpdate&date=%3E%3D2017-03-27T22%3A16%3A00.000Z&date=%3C2017-04-03T22%3A16%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_sort=-date&page=1#reports
Assignee: nobody → tschneider
Comment on attachment 8854682 [details]
Bug 1353529 - Crash when using IntersectionObserver in XUL pages.

https://reviewboard.mozilla.org/r/126640/#review129192

::: dom/base/DOMIntersectionObserver.cpp:284
(Diff revision 1)
>        }
>      }
>    } else {
>      nsCOMPtr<nsIPresShell> presShell = aDocument->GetShell();
>      if (presShell) {
>        rootFrame = presShell->GetRootScrollFrame();

Couldn't this also return null?
Attachment #8854682 - Flags: review?(mstange) → review+
It would if directly used in a XUL only document. We don't support that and would pass the intersection test by checking for null value later in the file. Our issue here was that an HTML document was embedded in XUL via an iframe, so that call returned the iframe's root scroll element but crashed when trying to walk up the presContext chain.
Keywords: checkin-needed
can't land this via autolander because of one open issue it seems. Can you take a look, thanks!
Flags: needinfo?(tschneider)
Keywords: checkin-needed
I've dropped the issue, can you try again? Thanks.
Flags: needinfo?(tschneider) → needinfo?(cbook)
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a628de1a8a06
Crash when using IntersectionObserver in XUL pages. r=mstange
(In reply to Markus Stange [:mstange] from comment #5)
> I've dropped the issue, can you try again? Thanks.

done
Flags: needinfo?(cbook)
https://hg.mozilla.org/mozilla-central/rev/a628de1a8a06
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Aurora approval on this when you get a chance. Might as well have it already landed by the time you're looking to re-enable there :)
Flags: needinfo?(tschneider)
Flags: in-testsuite+
Flags: needinfo?(bugs)
Comment on attachment 8854682 [details]
Bug 1353529 - Crash when using IntersectionObserver in XUL pages.

Approval Request Comment
[Feature/Bug causing the regression]: 1243846
[User impact if declined]: Crashes in popular RSS reader add-ons
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Is the change risky?]: No
Flags: needinfo?(tschneider)
Attachment #8854682 - Flags: approval-mozilla-beta?
Blocks: 1362168
Comment on attachment 8854682 [details]
Bug 1353529 - Crash when using IntersectionObserver in XUL pages.

Fix a crash when using IntersectionObserver. Beta54+. Should be in 54 beta 6.
Attachment #8854682 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(bugs)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.