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 content bounds are incorrectly resized #31513

Closed
3 tasks done
andreasdj opened this issue Oct 21, 2021 · 5 comments · Fixed by #32747
Closed
3 tasks done

[Bug]: BrowserView content bounds are incorrectly resized #31513

andreasdj opened this issue Oct 21, 2021 · 5 comments · Fixed by #32747

Comments

@andreasdj
Copy link
Contributor

andreasdj commented Oct 21, 2021

Preflight Checklist

Electron Version

15.2.0

What operating system are you using?

Windows

Operating System Version

Windows 10 enterprise, 21H1

What arch are you using?

x64

Last Known Working Electron version

13.2.1

Expected Behavior

When moving a child view partly outside of it's parents window bounds I expect it to keep it's size and position.

Actual Behavior

When moving a child view partly outside of it's parent window it's bounds are resized.

Testcase Gist URL

https://gist.github.com/900f0a593260572e8292e6db0e71f8de

Additional Information

The change was introduced between electron versions 13.2.1 and 13.2.2 (which can be tested using the fiddle). It's likely due to the change included inside the PR: #30540

Our use case is that we have a virtual keyboard and logic to shift a browser view position to fit focused elements inside the visible area when the keyboard is opened. We are shifting the brower view by setting a negative y-position for the bounds, as seen in the provided fiddle.

The new behavior with a resized viewport will affect the sizing and positioning of elements inside the viewport.

@sukhorukov
Copy link

Are there any updates on this? This issue is preventing us from updating to the latest electron version.

@andreasdj
Copy link
Contributor Author

@codebytere , do you have any feedback regarding this issue? I wonder if we could do anything to help out with investigating the issue described in #29778 which introduced the breaking change to see if there could be alternative ways of solving that transparency issue.

@weifeiyue, if the issue was resolved for you by resizing the bounds of the BrowserView to fit inside the parent window, then it seems like you can do that within your own code without relying on the electron resizing the view (which seems like a drastic way of fixing the issue).

@sukhorukov
Copy link

@sofianguy is there any chance that this will be fixed? Would it help if we make a PR with the fix?

@andreasdj
Copy link
Contributor Author

We have tested to download the electron source code and then build it using the electron build tools and can confirm that the change inside the PR: #30540 is causing the issue.

By reverting the change as seen here: https://github.com/electron/electron/pull/30540/files#diff-78a0440b35fe6e728fc0ef26b374d034e0b297634eb11368d19a4b44642f2902L218 and then rebuilding electron, it all work as expected.

This was tested by building the code from the main branch (i.e. the nightly releases and hence electron 18.0.0-nightly.20220201) so this issue will always remain as long as that change is not reverted.

We realize that the change was done to solve another issue but while it's intentions was to solve a transparency issue it's causing side effects I don't think was accounted for.

@andreasdj andreasdj changed the title [Bug]: Negative BrowserView content bounds are clipped [Bug]: BrowserView content bounds are incorrectly resized Feb 2, 2022
@andreasdj
Copy link
Contributor Author

When testing the gist provided in the original issue (#29778) which was the reason for introducing the bug it seems like the original issue still exists.

I tested the gist using electron fiddle with electron 17.0.0 and 16.0.8 and also in a local project with 18.0.0-nightly.20220201 (although with small modifications to add the BrowserViews after the main window was ready to be shown, otherwise the BrowserViews weren't rendered at all).

Can we do anything to make you consider reverting the single line change in the PR: #30540 @zcbenz, @codebytere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants