mattermost / desktop

Mattermost Desktop application for Windows, Mac and Linux
Apache License 2.0
2k stars 812 forks source link

Windows Client appears not to Size Main Window Client Region Appropriately on Windows 10 across monitors of different DPI #357

Closed wpostma closed 7 years ago

wpostma commented 7 years ago

I confirm (by marking "x" in the [ ] below):


Summary

The mattermost desktop windows application does not size its content correctly, on Windows 10 systems, with multiple monitors, and each monitor set to a different DPI scale.

I am a Windows developer, and can perhaps help you with what seems to be an Electron/native wrapper top level window size and content alignment issue in the Mattermost windows desktop app.

Steps to reproduce

Using mattermost desktop 3.x:

Expected behavior

On both 96 dpi (normal) and non 96 dpi (high dpi) systems, with both single monitors, and multiple monitors, in Windows 10, the content of the main window should be sized to fit the frame of the mattermost desktop window.

Note that Windows 10 supports different DPI values per monitor. The bug being reported may be an effect of per-monitor-dpi-awareness settings in the application manifest for mattermost.exe.

Observed behavior

Works on 96 dpi and single monitor high dpi systems. On non 96 dpi (high dpi) multi-monitor systems in Windows 10, the content of the main window is not properly sized to fit the frame of the mattermost desktop window.

The behaviour of desktop app 3.4.1 is actually worse than version 1.x of the desktop app.

Possible fixes

I am studying but I suspect it has to do with manifests. I am attaching an external manifest file that I am using to try to figure out what settings make the app "just work" on windows 10 multi monitor systems, with a mix of various DPI settings per monitor.

The Mattermost.manifest file should be placed in the same directory as mattermost.exe. Mattermost-custom-manifest.zip

The attached file makes the behaviour at least as good as 1.x, and is much improved for me.

wpostma commented 7 years ago

Attached manifest also shown here for discussion:

<?xml version="1.0" encoding="utf-8"?>
<asmv1:assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1" xmlns:asmv1="urn:schemas-microsoft-com:asm.v1" xmlns:asmv2="urn:schemas-microsoft-com:asm.v2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <assemblyIdentity version="1.0.0.0" name="MyApplication.app"/>

  <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
    <application>
          <!-- win7 -->
          <supportedOS Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}"/> 
    </application>
  </compatibility>

  <!-- per monitor DPI awareness -->
  <asmv3:application xmlns:asmv3="urn:schemas-microsoft-com:asm.v3">
    <asmv3:windowsSettings
         xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">
      <dpiAware>True/PM</dpiAware>
    </asmv3:windowsSettings>
  </asmv3:application>

   <!-- Windows 7 users who want modern Win7 file open dialogs -->
   <dependency>
    <dependentAssembly>
      <assemblyIdentity
        type="win32"
        name="Microsoft.Windows.Common-Controls"
        version="6.0.0.0"
        processorArchitecture="*"
        publicKeyToken="6595b64144ccf1df"
        language="*"
        />
    </dependentAssembly>
   </dependency>

</asmv1:assembly>
wpostma commented 7 years ago

The actual problem, if the issue didn't make it clear, is easier to see with a screenshot:

mm-problem

This also could be an issue where the top level window needs to cause a CSS layout reflow, and this reflow does not occur when the window first opens. An electron fix that causes a reflow of the CSS at startup, once the window is restored to its saved (maximized) initial position, might also be needed.

Currently to work around it I resize the window (restore from maximized to non-maximized, then re-maximize) and that was enough in mattermost 1.x. In mattermost 3.x, that workaround no longer works, and I have to resort to a custom manifest, PLUS the resize hack.

wpostma commented 7 years ago

This bug might be an upstream bug in Electron. If anyone knows if it's already a known problem on Electron not properly working in a Windows 10 per monitor DPI aware system, with multiple monitors, this could be closed and just linked to upstream Electron issues. If working around it by shipping a manifest with the product could be useful, I'm happy to help contribute one.

yuya-oc commented 7 years ago

As electron/electron#5429, it seems that per moniter DPI awareness is not supported in Electron.

If the workaround by manifest would work fine, we can discuss it. gif is very helpful for testers and readers to understand exact behavior. And just for reference, would you inspect DOM? Probably webview is correctly expanded, but the rendered region is incorrect.

jasonblais commented 7 years ago

Hey @wpostma,

Desktop app v3.5 had some improvements for high DPI systems, including Windows 10. The fix in question was for something else than reported here, but I'm wondering if it helped at all.

Would you like to try downloading the newest version and see if it helps resolve any of the issues you reported?

wpostma commented 7 years ago

Tested works great now! Can close this On Wed, Dec 14, 2016 at 10:29 PM Jason Blais notifications@github.com wrote:

Hey @wpostma https://github.com/wpostma,

Desktop app v3.5 had some improvements for high DPI systems, including Windows 10. The fix in question was for something else than reported here, but I'm wondering if it helped at all.

Would you like to try downloading the newest version https://about.mattermost.com/download/#mattermostApps and see if it helps resolve any of the issues you reported?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/mattermost/desktop/issues/357#issuecomment-267229301, or mute the thread https://github.com/notifications/unsubscribe-auth/ABv5_o6T84RwcPtwJnjs-BCACFn1EMBgks5rILQIgaJpZM4KnYlv .

jasonblais commented 7 years ago

Glad to hear!

yuya-oc commented 7 years ago

I could also confirm that the issue doesn't reproduce with v3.5.