Closed Bug 1685038 Opened 5 years ago Closed 3 years ago

ResizeObserver generates bogus error messages if in iframe

Categories

(Core :: Layout, defect)

Firefox 84
defect

Tracking

()

RESOLVED FIXED
105 Branch
Tracking Status
firefox84 --- wontfix
firefox85 --- wontfix
firefox86 --- wontfix
firefox94 --- wontfix
firefox105 --- fixed

People

(Reporter: mozilla2, Assigned: dshin)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36

Steps to reproduce:

  • Unzip the files
  • open outer.html
  • open the JSConsole (Ctrl-Shift-K/Cmd-Shift-K)
  • Drag the bottom edge of the window up and down

You can use a live version here

https://greggman.github.io/doodles/test/firefox-resizeobserver-error-issue/outer.html

Actual results:

There is the error "ResizeObserver loop completed with undelivered notifications."

Expected results:

There should be no errors

relevant contents of files

outer.html
    <iframe src="frame.html">

frame.html
    <iframe src="resize.html>

resize.html
    <body>
      <canvas id="c"></canvas>
      <div id="info"></div>
      <div id="arrow">drag this ⬇ slider up/down</div>
    </body>
    <script>
    const canvas = document.querySelector('#c');

    function onResize(entries) {}

    const resizeObserver = new ResizeObserver( onResize );
    resizeObserver.observe( canvas, { box: "content-box" } );

    /* if you don't check for the error non get report!?!? */
    window.addEventListener('error', function(e) {
      console.error(e.error || e);
    });
    </script>
  • If aren't 2 nested iframes no error message.

    In other words if you open resize.html or frame.html directly then no error message

  • If you remove the error event listener then no error message

    That's strange because without the error event listener the error should go to the console automatically

Tested on Mac and Windows, Also tested on Nightly 86.0a1 (2021-01-04) (64-bit)

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → Layout
Product: Firefox → Core

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:84.0) Gecko/20100101 Firefox/84.0

Hi,

I have managed to reproduce the issue in release 84.0.2, beta 85.0b6 and latest nightly 86.0a1 (2021-01-07). The issue is also reproducible in Fx70 (build id: 20190804214813).

Thanks for the report.

Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Blocks: 1272409
Blocks: resize-observer-1
No longer blocks: 1272409
Assignee: nobody → boris.chiou

Should not block the spec meta bug.

No longer blocks: resize-observer-1
Blocks: 1732237

This is also causing issue for us in noVNC as we assume that error events means some form of page error and hence display a crash popup for the user when this happens.

Given that this event is fired without any useful details, and that it isn't logged in the console, gives the impression that this error event isn't entirely thought through.

Also noteworthy that this issue does not seem to exist in Chrome. I don't get a hit for this error message in Chrome's source code, so perhaps they never fire this error event?

I don't see the error message in nightly at least?

I still see the error in nightly 99.0a1 (2022-02-24) (64-bit). Tested both MacOS and Windows 10

https://i.imgur.com/6736MbH.png

Did a quick rr on this - Seems that when you move the bottom edge of the window around, in the observer loop:

  • GatherAllActiveObservations called with shallowestTargetDepth = 0 (Line 120)
  • Changes broadcasted, target depth updated, last reported size updated (Line 124)
  • Flush layout, window size changes again (Line 132, presumably)
  • GatherAllActiveObservations called with shallowestTargetDepth now set to the canvas depth - size changed again, but depth of canvas is not deeper than shallowestTargetDepth, the change is skipped (As per the standard)
  • Eventually we exit the loop, figure out that there are skipped observations, then send out the error (As per the standard)

Not sure why this only happens 2 iframes deep.

Assignee: boris.chiou → dshin

Hmm. Interesting. Pernosco session here: https://pernos.co/debug/vgSf1FQhOxoJ2sGSiEPi1g/index.html#f{m[Cf7t,4vs_,t[bg,B/1c_,f{e[Cf7t,4aU_,s{af41OAkAA,bAXs,uD4gjWQ,oD4uZsQ___/

  • Main window gets resized by receiving the resize event, which triggers an immediate reflow (nsViewManager::SetWindowDimensions () at nsViewManager.cpp:216)
  • The outermost iframe sets a delayed resize for its content (nsSubDocumentFrame::ReflowFinished () at nsSubDocumentFrame.cpp:764)
  • Next refresh tick happens
  • In the style flush of the tick, the outermost iframe's delayed resize gets resolved
  • In the layout flush of the tick, the middle iframe sets a delayed resize for its content
  • In the paint flush of the tick, resize observer runs the first loop
  • The resize observer flushes layout
  • The middle iframe's delayed resize gets resolved for its content. The inner iframe's div gets resized due to this.
  • The next cycle of resize observer skips an observation, causing this error.

Basically seems that the cause is subdocuments (embed, object, iframe) doing a delayed resize of its content where the resize seems to get resolved the next cycle. When it nests once, the immediate reflow of the main window causes a reflow, which sets the delayed reflow, and since that delayed reflow gets resolved in the layout flush of the tick before the paint flush where resize observers run, we avoid the errors.

I couldn't reproduce this, but this matches the specification.

https://drafts.csswg.org/resize-observer/#html-event-loop:

  1. Recalc styles
  2. Update layout
  3. Set depth to 0
  4. Gather active observations at depth depth for Document
  5. Repeat while document has active observations
  6. Set depth to broadcast active observations.
  7. Recalc styles
  8. Update layout
  9. Gather active observations at depth depth for Document

This matches the spec, tweaked a bit to avoid duplication.

I haven't been able to reproduce the original issue, but flushing
upfront should make sure that delayed resizes are processed given we
flush the browsing context tree in pre-order.

Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/77c11ac6b6a0 Flush layout _before_ gathering observations in ResizeObserverController. r=dshin
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 105 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: