Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[resize-observer] The initial size of ResizeObservation #3664

Open
cathiechen opened this issue Feb 20, 2019 · 7 comments
Open

[resize-observer] The initial size of ResizeObservation #3664

cathiechen opened this issue Feb 20, 2019 · 7 comments

Comments

@cathiechen
Copy link

https://drafts.csswg.org/resize-observer/#dom-resizeobservation-resizeobservation-target-options

  1. Set this internal lastReportedSize slot to (0,0)

Not very strong opinion about this.
We might miss this:

  • Element's size isn't (0, 0)
  • ro.observe(Element)
  • Element's size change to (0, 0)
@cathiechen
Copy link
Author

Add a case to reproduce this issue.
https://jsbin.com/qotevalowe/1/edit?html,console,output

@mrego
Copy link
Member

mrego commented Feb 22, 2019

Add a case to reproduce this issue.
https://jsbin.com/qotevalowe/1/edit?html,console,output

In that example if you remove the last line (the one that changes the size to 0x0), then you'll get notified for a size change of 100x100 (when nothing has actually changed). I guess it's because the initial lastReportedSize is 0x0 and it detects a change.

@cathiechen
Copy link
Author

@mrego Yes, exactly like you said. :) We might also get notified when nothing has changed.

@atotic
Copy link
Contributor

atotic commented Mar 12, 2019

There was a discussion about what initial size should be at:
WICG/resize-observer#8

Options were:

  • current size of the element
    • rejected because this might cause RO.observe() to trigger layout.
  • (0,0)
    • causes RO to fire immediately after being observed, unless Element size is 0,0
    • 0,0 is also Element's size when hidden. RO will not fire initially until Element is shown.
  • undefined
    • would cause RO to always fire, even when hidden.

I think (0,0) option is most useful, because it does not fire until Element is shown.

@Loirooriol
Copy link
Contributor

Loirooriol commented Aug 4, 2022

This is inconsistent with IntersectionObserver, which always delivers a notification just after observing, since

  1. When observing a target Element, previousThresholdIndex is set to -1.
  2. When running the update intersection observations steps, thresholdIndex is set to a non-negative index:

    Set thresholdIndex to the index of the first entry in observer.thresholds whose value is greater than intersectionRatio, or the length of observer.thresholds if intersectionRatio is greater than or equal to the last entry in observer.thresholds.

  3. This ensures a IntersectionObserverEntry will be queued:

    If thresholdIndex does not equal previousThresholdIndex or if isIntersecting does not equal previousIsIntersecting, queue an IntersectionObserverEntry

Similarly, I think that ResizeObservation should initialize lastReportedSizes to something invalid like [(-1,-1)], so that we consistently get a notification regardless of the current size.

contain-intrinsic-size: auto also needs something like this, it uses an internal ResizeObserver to store the last remembered size. And 0x0 is a valid size that should still be stored, so the callback should be invoked. See #7539

I think (0,0) option is most useful, because it does not fire until Element is shown.

I don't think this justifies not reporting an observation for elements that do have a box but it just happens to be 0x0. Also note IntersectionObserver notifies about disconnected elements:

new IntersectionObserver(console.log).observe(document.createElement("div"));

@emilio emilio added the Agenda+ label Aug 11, 2022
@emilio
Copy link
Collaborator

emilio commented Aug 11, 2022

Agenda+ to confirm the resolution in #7539 (comment) applies to ResizeObserver too (or get a new one otherwise), since from discussion with @Loirooriol it wasn't quite clear from the minutes.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [resize-observer] The initial size of ResizeObservation, and agreed to the following:

  • RESOLVED: When first observing an element with ResizeObserver, lastReportedSize gets initialized with a -1 x -1 size
The full IRC log of that discussion <emeyer> Topic: [resize-observer] The initial size of ResizeObservation
<emeyer> github: https://github.com//issues/3664
<emeyer> emilio: FTF resolved that zero was not a special size for contain intrinsic size
<emeyer> …The resolution seems to include resizeObserver, but in chats with Oriol it evolved that wasn’t clear
<TabAtkins> +
<TabAtkins> q+
<emeyer> …Worth clarifying
<Rossen_> ack TabAtkins
<emeyer> TabAtkins: The resolution was not intended to apply to resizeObserver but I’m fine with it applying
<emeyer> …Consistency with things like c-i-s has me leaning to fire immediately and have an undefined size in resizeObserver
<emeyer> Rossen_: Any objections to the proposed resolution?
<emilio> When observing an element with ResizeObserver, lastReportedSize gets initialized with a -1x-1 size
<emeyer> RESOLVED: When first observing an element with ResizeObserver, lastReportedSize gets initialized with a -1 x -1 size

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 30, 2022
…lly. r=emilio

It used to be 0x0, but changed per the CSSWG resolution:
w3c/csswg-drafts#3664
Sits behind pref `dom.resize_observer.last_reported_size_invalid`.

Differential Revision: https://phabricator.services.mozilla.com/D155710
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 1, 2022
It used to be 0x0, but changed per the CSSWG resolution:
w3c/csswg-drafts#3664
Sits behind pref `dom.resize_observer.last_reported_size_invalid`.

Differential Revision: https://phabricator.services.mozilla.com/D155710

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1785305
gecko-commit: 14e81415cae10e0699145eb052b49d369f5ee427
gecko-reviewers: emilio
jgraham pushed a commit to web-platform-tests/wpt that referenced this issue Sep 22, 2022
It used to be 0x0, but changed per the CSSWG resolution:
w3c/csswg-drafts#3664
Sits behind pref `dom.resize_observer.last_reported_size_invalid`.

Differential Revision: https://phabricator.services.mozilla.com/D155710

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1785305
gecko-commit: 14e81415cae10e0699145eb052b49d369f5ee427
gecko-reviewers: emilio
cathiechen added a commit to cathiechen/WebKit that referenced this issue Jan 23, 2023
…e -1x-1

https://bugs.webkit.org/show_bug.cgi?id=250836

Reviewed by NOBODY (OOPS!).

The conclusion of [1] indicates that when first observing an element with ResizeObserver,
lastReportedSize gets initialized with a -1 x -1 size.

[1] w3c/csswg-drafts#3664

* LayoutTests/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/resize-observer/observe-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/resize-observer/svg-expected.txt:
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt: Copied from LayoutTests/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt.
* LayoutTests/platform/mac/imported/w3c/web-platform-tests/resize-observer/svg-expected.txt:
* Source/WebCore/page/ResizeObservation.cpp:
webkit-early-warning-system pushed a commit to cathiechen/WebKit that referenced this issue Feb 1, 2023
…should be -1x-1 which always notify observed elements

https://bugs.webkit.org/show_bug.cgi?id=251015

Reviewed by Oriol Brufau and Ryosuke Niwa.

This patch initializes the lastReportedSize of ResizeObservation to -1 x -1, which is the conclusion of [1],
it indicates that -1 x -1 is the initial size when first observing an element with ResizeObserver.

The previous change makes every ResizeObservation be delivered at least once, even if the target is a disconnected element.
When GC happens between observe and gather ResizeObservations, JSC::Weak<JSC::JSObject> m_callback inside JSCallbackDataWeak
could be released, and the element and JSResizeObserver might not be released, for they are not strong connected, this scenario
would trigger the ASSERT in ResizeObserver::deliverObservations. To fix it, this patch adds m_targetsWaitingForFirstObservation
to extend the life cycle of ResizeObserver to the first time of deliverObservations. The fix is similar to
what IntersectionObserver does in bug 231235.

The test6 of resize-observer/notify.html still fails in mac-wk1, because updaterendering is not triggered properly.
Filed [2] to track it.

[1] w3c/csswg-drafts#3664
[2] https://bugs.webkit.org/show_bug.cgi?id=250900

* LayoutTests/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/resize-observer/observe-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/resize-observer/svg-expected.txt:
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt: Copied from LayoutTests/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt.
* LayoutTests/platform/mac/imported/w3c/web-platform-tests/resize-observer/svg-expected.txt:
* Source/WebCore/page/ResizeObservation.cpp:
* Source/WebCore/page/ResizeObserver.cpp:
(WebCore::ResizeObserver::observe):
(WebCore::ResizeObserver::deliverObservations):
(WebCore::ResizeObserver::isReachableFromOpaqueRoots const):
(WebCore::ResizeObserver::removeAllTargets):
(WebCore::ResizeObserver::removeObservation):
* Source/WebCore/page/ResizeObserver.h:

Canonical link: https://commits.webkit.org/259673@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants