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

Setting browser window position causes incorrect default window options to be set #345

Closed
ddaspit opened this issue Nov 29, 2019 · 5 comments
Assignees
Labels

Comments

@ddaspit
Copy link

ddaspit commented Nov 29, 2019

  • Version: 5.30.1
  • Target: Windows 10

If the X and Y properties are set when creating a browser window, then some of the default values for properties in BrowserWindowOptions are set incorrectly. The most noticeable issue is that the default font size is no longer correct (16) and set to a very small value (6). This occurs because the CreateWindowAsync method in WindowManager disables ignoring default values during serialization when X and Y are set. Many of the primitive properties in BrowserWindowOptions and WebPreferences (such as DefaultFontSize) do not have the correct default values set. A possible solution might be to re-enable ignoring default values and adding [DefaultValue(-1)] attribute to the X and Y properties in BrowserWindowOptions.

Also, when setting the position of the window on Windows 10, CreateWindowAsync subtracts 7 from the X property. This seems to be added to correct for a bug in Electron. On my Windows 10 machine, this results in the window being incorrectly positioned too far to the left. For example, if I set the X to 0, a small amount of the left side of the window is off of the screen. If I set X to 7, the window is positioned exactly on the left side of the screen. It might be worth checking if the Electron bug is still an issue. Right now, I have to add 7 to X in order to get the correct position.

Steps to Reproduce:

  1. Set X and Y in the BrowserWindowOptions when calling CreateWindowAsync.
  2. Start the app.
  3. The default font size is too small in the browser window.
@ddaspit ddaspit added the bug label Nov 29, 2019
@GregorBiswanger GregorBiswanger self-assigned this Dec 3, 2019
@GregorBiswanger
Copy link
Member

I have tried it with the latest Electron.NET version (7.30.2). For me, the window position works and the font stays in the default size.

Task.Run(async () =>
{
    var browserWindowOptions = new BrowserWindowOptions 
    {
        X = 0,
        Y = 0
    };

    await Electron.WindowManager.CreateWindowAsync(browserWindowOptions);
});

image

@ddaspit
Copy link
Author

ddaspit commented Dec 4, 2019

I forked and updated the music player sample app to demonstrate the issue. I set the X and Y properties to 0 in the issue345 branch. With the X and Y properties set, I get:
issue345
Without the X and Y properties set (the master branch), I get:
master

@GregorBiswanger
Copy link
Member

Fixed and use the new Electron.NET 8.31.1.

@duncanawoods
Copy link

@GregorBiswanger I'm seeing this problem in 9.31.2.0. If I set the browser window size, fonts using rem become tiny. Can you please point me towards the cause and the commit that fixed it last time and I'll try to fix it?

@duncanawoods
Copy link

@GregorBiswanger The problem in WindowManager.cs, if the position is set then it uses a different serialiser for the BrowserWindowOptions:

var ownjsonSerializer = new JsonSerializer()
{
    ContractResolver = new CamelCasePropertyNamesContractResolver(),
    NullValueHandling = NullValueHandling.Ignore
};

However, not all default values are set:

/// <summary>
/// Defaults to 16.
/// </summary>
public int DefaultFontSize { get; set; }

Setting this value seems to fix the issue but we need to 100% certain all the option defaults are correct.

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

No branches or pull requests

3 participants