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.

Sign up for GitHub

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

Jump to bottom

[ResizeObserver] The initial value of the last reported size should be -1x-1 #8839

Closed
wants to merge 1 commit into from
Closed

[ResizeObserver] The initial value of the last reported size should be -1x-1 #8839

wants to merge 1 commit into from

Conversation

cathiechen
Copy link
Contributor

@cathiechen cathiechen commented Jan 19, 2023 •
edited by webkit-early-warning-system

91f24ff

[ResizeObserver] The initial value of the last reported size should be -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:

91f24ff

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@cathiechen cathiechen self-assigned this Jan 19, 2023
@cathiechen cathiechen added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Jan 19, 2023
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash ac99702)

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ❌ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@Loirooriol
Copy link
Contributor

The commit message seems wrong.

The bug for this is https://bugs.webkit.org/show_bug.cgi?id=250836 instead of https://bugs.webkit.org/show_bug.cgi?id=157743
And the link should point to w3c/csswg-drafts#3664 instead of https://bugs.webkit.org/show_bug.cgi?id=250836

@cathiechen cathiechen mentioned this pull request Jan 19, 2023
Merged
Loirooriol
Loirooriol approved these changes Jan 19, 2023
View reviewed changes
Copy link
Contributor

@Loirooriol Loirooriol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if you fix the commit message

@cathiechen
Copy link
Contributor Author

The commit message seems wrong.

The bug for this is https://bugs.webkit.org/show_bug.cgi?id=250836 instead of https://bugs.webkit.org/show_bug.cgi?id=157743 And the link should point to w3c/csswg-drafts#3664 instead of https://bugs.webkit.org/show_bug.cgi?id=250836

Ah, indeed! Fixed, thanks a lot!

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 19, 2023
Loirooriol
Loirooriol reviewed Jan 23, 2023
View reviewed changes
Copy link
Contributor

@Loirooriol Loirooriol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You edited the comment, but the commit was not updated. I suspect this won't do.

Loirooriol
Loirooriol requested changes Jan 23, 2023
View reviewed changes
Copy link
Contributor

@Loirooriol Loirooriol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess observe-disconnected-target-crash.html fails because you need something like 1a04d71

@cathiechen
Copy link
Contributor Author

You edited the comment, but the commit was not updated. I suspect this won't do.

I will try to upload a new commit, see if it works:)

@cathiechen
Copy link
Contributor Author

I guess observe-disconnected-target-crash.html fails because you need something like 1a04d71

Thanks, I'll take a look

@cathiechen
[ResizeObserver] The initial value of the last reported size should b…
91f24ff 
…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:
@cathiechen cathiechen removed the merging-blocked Applied to prevent a change from being merged label Jan 23, 2023
@cathiechen cathiechen force-pushed the eng/ResizeObserver-The-initial-value-of-the-last-reported-size-should-be--1x-1 branch from ac99702 to 91f24ff Compare January 23, 2023 17:16
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 91f24ff)

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 23, 2023
@cathiechen cathiechen mentioned this pull request Jan 25, 2023
Merged
@denschub denschub mentioned this pull request Jan 31, 2023
Closed
@cathiechen cathiechen closed this Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Loirooriol Loirooriol Loirooriol requested changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@smfr smfr Awaiting requested review from smfr

@heycam heycam Awaiting requested review from heycam

Assignees

@cathiechen cathiechen

Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing). merging-blocked Applied to prevent a change from being merged
Projects
None yet
Milestone
No milestone
4 participants
@cathiechen @webkit-early-warning-system @Loirooriol @webkit-ews-buildbot