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

[Bug]: BrowserView.setBounds is not appliead immediately on first calls. #39993

Closed
3 tasks done
marekharanczyk opened this issue Sep 27, 2023 · 1 comment · Fixed by #39994
Closed
3 tasks done

[Bug]: BrowserView.setBounds is not appliead immediately on first calls. #39993

marekharanczyk opened this issue Sep 27, 2023 · 1 comment · Fixed by #39994
Labels

Comments

@marekharanczyk
Copy link
Contributor

Preflight Checklist

Electron Version

28, 27, 26 and older

What operating system are you using?

Windows

Operating System Version

Windows 10

What arch are you using?

x64

Last Known Working Electron version

Probably never

Expected Behavior

setBounds applied immediately, BrowserView painted in new position.

Actual Behavior

setBounds sets the data but view is still being painted with old bounds even though getBounds returns new bounds.

Testcase Gist URL

https://gist.github.com/2e8b6adb97bf44893828212a79b06b22

Additional Information

Setting same bounds twice will make BrowserView paint correctly. This is because same calling setBounds with same rect immediately triggers layout call on Views (https://source.chromium.org/chromium/chromium/src/+/ba15ffd71335c75897f4bd8501b3c8ed00a284da:ui/views/view.cc;l=386), while different bounds will make it schedule InvalidateLayout call, which is propagated to parent and view is marked as needing layout, expecting to receive it from parent on next layout call . The problem is that BrowserView's view is added as child of InspectableWebContentsViews which does not call setBounds (which triggers layout) on all of it's children when doing it's layout, so it skips propagating Layout call to its children BrowserViews views, even though those were marked as needing layout.

marekharanczyk added a commit to marekharanczyk/electron that referenced this issue Sep 27, 2023
…ews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes electron#39993.
jkleinsc pushed a commit that referenced this issue Sep 28, 2023
…ViewViews` (#39994)

Propagate layout call to all children of InspectableWebContentsViewViews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes #39993.
trop bot added a commit that referenced this issue Sep 28, 2023
…ews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes #39993.

Co-authored-by: Marek Haranczyk <marek@openfin.co>
trop bot added a commit that referenced this issue Sep 28, 2023
…ews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes #39993.

Co-authored-by: Marek Haranczyk <marek@openfin.co>
trop bot added a commit that referenced this issue Sep 28, 2023
…ews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes #39993.

Co-authored-by: Marek Haranczyk <marek@openfin.co>
jkleinsc pushed a commit that referenced this issue Sep 28, 2023
…ViewViews` (#40036)

Propagate layout call to all children of InspectableWebContentsViewViews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes #39993.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Marek Haranczyk <marek@openfin.co>
jkleinsc pushed a commit that referenced this issue Sep 28, 2023
…ViewViews` (#40037)

Propagate layout call to all children of InspectableWebContentsViewViews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes #39993.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Marek Haranczyk <marek@openfin.co>
codebytere pushed a commit that referenced this issue Sep 29, 2023
…ViewViews` (#40035)

Propagate layout call to all children of InspectableWebContentsViewViews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes #39993.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Marek Haranczyk <marek@openfin.co>
@FrankenApps
Copy link

This was previously reported in #35149, but unfortunately closed due to inactivity in the meantime...

MrHuangJser pushed a commit to MrHuangJser/electron that referenced this issue Dec 11, 2023
…ViewViews` (electron#39994)

Propagate layout call to all children of InspectableWebContentsViewViews.

When BrowserView bounds are set from js, those might not trigger layout
immediately, sometimes propagating InvalidateLayout call to parent.
View is marked as needing layout, expecting to receive it from parent on
next layout call. The problem is that BrowserView's view is added as child
of InspectableWebContentsViews which does not call setBounds (which
would trigger layout) on all of it's children when doing it's layout,
so it skips propagating Layout call to its children BrowserViews views,
even though those were marked as needing layout.
Call base class View::Layout which will iterate over views' children
and call Layout on those that were marked as needing them.

Fixes electron#39993.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants