quasarframework / quasar

Quasar Framework - Build high-performance VueJS user interfaces in record time
https://quasar.dev
MIT License
25.96k stars 3.52k forks source link

Screen plugin reports incorrect values on cordova WKWebView #4950

Closed GordonBlahut closed 4 years ago

GordonBlahut commented 5 years ago

Describe the bug iOS WKWebView seems to have a bug with reporting wrong window.innerHeight and window.innerWidth in the resize event when the device is rotated. The values are stale. It eventually ends up reporting a square screen with the smaller of the 2 values. This in turn causes the Screen plugin to report incorrect dimensions, which then cascades to every other area that depends on it (like QPage setting minimum height inside a QLayout).

WKWebView does end up with the correct values at some point, just not in time for the resize event so Screen plugin never sees them.

WKWebView does seem to immediately report correct values for document.documentElement.clientHeight and document.documentElement.clientWidth so maybe those values could be used for mobile devices/cordova mode since they don't have scrollbars that affect layout anyway. Probably wouldn't want to use these values on devices that do have scrollbars though since the values will differ from window.innerHeight and window.innerWidth.

Codepen/jsFiddle/Codesandbox (required) The issue is specific to cordova mode so a codepen is not helpful. See screenshots.

Adding this to a template will help demonstrate though:

    Height: {{ $q.screen.height }}<br>
    Width: {{ $q.screen.width }}

To Reproduce Steps to reproduce the behavior:

  1. Enable cordova suport for a quasar app.
  2. Add cordova-plugin-ionic-webview or cordova-plugin-wkwebview-engine cordova plugin
  3. Run the app
  4. Rotate the device from portrait to landscape
  5. Rotate the device from landscape to portrait
  6. See error

It may take several rotates before the issue appears.

Expected behavior Screen plugin to report correct width/height.

Screenshots https://imgur.com/a/eOeoDEr

Platform (please complete the following information): OS: Darwin(18.6.0) - darwin/x64 Node: 10.16.3 NPM: 6.9.0 Yarn: 1.17.3 Browsers: cordova with WKWebView iOS: 12.1, 12.2, probably others

Additional context I would also be ok with working around this in devland if there's a way to override the Screen plugin's behavior.

mesqueeb commented 5 years ago

Stumbled upon the same problem. ;)

mesqueeb commented 5 years ago

@GordonBlahut Did you have any manual workaround which involves detecting a rotation to update the min-height?

GordonBlahut commented 5 years ago

@mesqueeb Sorry, not at this time. I did have a proof of concept that overwrote the screen values but it seems to have gotten lost. If I find it again or re-implement I will be sure to share.

GordonBlahut commented 5 years ago

@mesqueeb Here is my current workaround. Seems to work ok (with cavaets below) but could be some issues I haven't come across yet.

Cavaets:

  1. I don't use SSR and don't see a way to get the "queues" reference from the original screen plugin. I removed the SSR logic from this custom version as a result. Sorry, if you use SSR this won't help you.
  2. If anything holds a reference to $q.screen before it can be replaced by the custom version, the default screen plugin will continue to be referenced (I had a different plugin that held a reference to screen, I just changed it to reference $q and get screen from $q instead). Not sure if anything built-in to quasar holds a direct reference to screen or not.
  3. The custom plugin does not check if it's in cordova ios or not, so you'd want to wrap the call to install inside a check for cordova ios. If you use it on other platforms like desktop, you will get different results due to scroll bars.
  4. There's no opting out of the default screen plugin, so the only thing I could figure out to prevent conflicts was to set its debounce delay to a really large number to prevent it from running on each resize (I set it to a year).
  5. This is far from an ideal solution, since there's a lot of code duplication from the original plugin. Hopefully the need for a workaround is very temporary.
  6. The code is provided as-is with no warranty, support, etc. You're on your own.

Custom version of screen plugin. I named mine ScreenWKWebViewCompat.js but you can call it whatever you want. Call the install method from a quasar boot file.

/* eslint-disable */
import Vue from 'vue'

import { debounce, event } from 'quasar'
const { listenOpts } = event

const SIZE_LIST = ['sm', 'md', 'lg', 'xl']

export default {
  width: 0,
  height: 0,

  sizes: {
    sm: 600,
    md: 1024,
    lg: 1440,
    xl: 1920
  },

  lt: {
    sm: true,
    md: true,
    lg: true,
    xl: true
  },
  gt: {
    xs: false,
    sm: false,
    md: false,
    lg: false
  },
  xs: true,
  sm: false,
  md: false,
  lg: false,
  xl: false,

  setSizes () {},
  setDebounce () {},

  install ($q) {

    let update = force => {
      if (document.documentElement.clientHeight !== this.height) {
        this.height = document.documentElement.clientHeight
      }

      const w = document.documentElement.clientWidth

      if (w !== this.width) {
        this.width = w
      }
      else if (force !== true) {
        return
      }

      const s = this.sizes

      this.gt.xs = w >= s.sm
      this.gt.sm = w >= s.md
      this.gt.md = w >= s.lg
      this.gt.lg = w >= s.xl
      this.lt.sm = w < s.sm
      this.lt.md = w < s.md
      this.lt.lg = w < s.lg
      this.lt.xl = w < s.xl
      this.xs = this.lt.sm
      this.sm = this.gt.xs && this.lt.md
      this.md = this.gt.sm && this.lt.lg
      this.lg = this.gt.md && this.lt.xl
      this.xl = this.gt.lg
    }

    let updateEvt, updateSizes = {}, updateDebounce = 16

    this.setSizes = sizes => {
      SIZE_LIST.forEach(name => {
        if (sizes[name] !== void 0) {
          updateSizes[name] = sizes[name]
        }
      })
    }
    this.setDebounce = deb => {
      updateDebounce = deb
    }

    const start = () => {
      const style = getComputedStyle(document.body)

      // if css props available
      if (style.getPropertyValue('--q-size-sm')) {
        SIZE_LIST.forEach(name => {
          this.sizes[name] = parseInt(style.getPropertyValue(`--q-size-${name}`), 10)
        })
      }

      this.setSizes = sizes => {
        SIZE_LIST.forEach(name => {
          if (sizes[name]) {
            this.sizes[name] = sizes[name]
          }
        })
        update(true)
      }

      this.setDebounce = delay => {
        const fn = () => { update() }
        updateEvt && window.removeEventListener('resize', updateEvt, listenOpts.passive)
        updateEvt = delay > 0
          ? debounce(fn, delay)
          : fn
        window.addEventListener('resize', updateEvt, listenOpts.passive)
      }

      this.setDebounce(updateDebounce)

      if (Object.keys(updateSizes).length > 0) {
        this.setSizes(updateSizes)
        updateSizes = void 0 // free up memory
      }
      else {
        update()
      }
    }

    start()

    // prevent default screen plugin from updating
    $q.screen.setDebounce(31556952000)

    // replace reference to default screen plugin
    $q.screen = this
  }
}
rstoenescu commented 4 years ago

Re-tested this with iOS 13 and it seems that they've fixed it. So no changes are necessary.

GordonBlahut commented 4 years ago

Does Quasar only support the latest iOS version? I'm stilling seeing issues with iOS 13.2.2 but I will try again with iOS 13.3 once I can update.

Unfortunately I can't force all users to upgrade to iOS 13 so I will have to continue using my workaround.

mesqueeb commented 4 years ago

@GordonBlahut try capacitor. I was able to switch from Cordova to capacitor without require any change (maybe just a few lines ) and all within two hours. This included me reading through the capacitor docs. It's really easy and works extremely well !

I found that on capacitor I had zero layout and screen height related problems. I think they do a better job integrating the wkwebview technology.

GordonBlahut commented 4 years ago

@rstoenescu Unfortunately I'm still seeing incorrect screen size issues even on iOS 13.3. I appreciate the bug is not within quasar itself but could there be some sort of way to hook into the screen plugin update process so it can be worked around in devland? Basically just need a way to specify the source for the width and height instead of forced window.inner(Height|Width).

@mesqueeb I have quite a few cordova plugins so I'll have to see how compatible they are with capacitor. It's interesting that you haven't had any issues with that since the company behind capacitor is also behind cordova-plugin-ionic-webview. I also have another issue with cordova-plugin-ionic-webview where iOS 12 jumps to the top of a page when the keyboard closes so if that doesn't happen in capacitor then I'll really have to focus on switching.

rstoenescu commented 4 years ago

If anyone has a proposal, please PR it.

rstoenescu commented 4 years ago

For the life of me I cannot reproduce.

Steps I followed:

  1. quasar create testapp
  2. quasar mode add cordova
  3. cd src-cordova; cordova plugin add cordova-plugin-ionic-webview; cd ..
  4. edited src-cordova/config.xml and added <preference name="ScrollEnabled" value="true" />
  5. quasar dev -m ios

Tried on emulator (iPhone 11 Pro Max with iOS 13.3) and on real device (iPhone 11 with iOS 13.3). Changed device orientation a million times. Width and height are reported correctly.

Are you sure that you are also running latest "quasar" (v1.5.9 as of writing these lines)?

rstoenescu commented 4 years ago

Also, are you sure that you are running latest "@quasar/app" too? (v1.4+)

GordonBlahut commented 4 years ago

@rstoenescu I really appreciate you spending time to take a look at this. I know how annoying it can be to not be able to reproduce something.

I am about to take a leave from work for the next 2 weeks so unfortunately I won't have access to a Mac or iOS devices until I return but I will try to create a PR that isn't ugly.

I am using:

But those versions shouldn't even matter because the source of the issue is within webkit itself. Here is a webkit bug going back to iOS 10: https://bugs.webkit.org/show_bug.cgi?id=170595. It only seems to affect wkwebview but not mobile Safari itself.

WKWebView does eventually report the correct window.innerHeight/window.innerWidth, but not in time for the resize or orientationchange events.

I didn't have <preference name="ScrollEnabled" value="true" />, but adding it does not appear to change anything. Scrolling seems to work the same regardless of that preference being true or false or not present.

Here is a sample index SFC

<template>
  <q-page class="flex flex-center">
    <div class="q-gutter-md">
      <div>
        $q.screen.height: {{ $q.screen.height }}<br />
        $q.screen.width: {{ $q.screen.width }}<br />
        clientHeight: {{ clientHeight }}<br />
        clientWidth: {{ clientWidth }}
      </div>
      <div>
        <q-btn label="refresh current window info" @click="display" />
      </div>
      <div>
        Last manual refresh:<br />
        window.innerHeight: {{ height }}<br />
        window.innerWidth: {{ width }}
      </div>
    </div>
  </q-page>
</template>

<script>
import { event } from "quasar"
const { listenOpts } = event

export default {
  name: 'PageIndex',
  data: function() {
    return {
      width: window.innerWidth,
      height: window.innerHeight,
      clientHeight: document.documentElement.clientHeight,
      clientWidth: document.documentElement.clientWidth
    }
  },
  methods: {
    display: function() {
      this.height = window.innerHeight;
      this.width = window.innerWidth;
    },
    updateSize: function() {
      this.clientHeight = document.documentElement.clientHeight;
      this.clientWidth = document.documentElement.clientWidth;
    }
  },
  created: function() {
    this.onResize = this.updateSize.bind(this)
    window.addEventListener("resize", this.onResize, listenOpts.passive)
  },
  beforeDestroy: function() {
    window.removeEventListener("resize", this.onResize, listenOpts.passive)
  }
}
</script>

The manual refresh button is just to show that window.innerHeight/Width are correct when checked ondemand outside of resize events. Note that document.documentElement.clientHeight/Width are correct on resize, but I believe these behave differently with scrollbars, and possibly the software keyboard, etc so it's not a simple drop in replacement for all platforms or modes.

And a short video: https://gfycat.com/carefreelittleindianpalmsquirrel. In this video, it really got stuck on 320 height for a while but does eventually get 568 again. I believe that's because I switched back and forth really quickly and it output a cached 568 width as the height. Also notice that the width seems to never change from 320, but a manual refresh shows that the source value is eventually correct at some point after the event ends.

The width being wrong doesn't seem to affect to much since I don't think much relies on it, but the height ends up affecting the min-height set on the q-page.

GordonBlahut commented 4 years ago

The video was recorded on a device running iOS 12.

I can longer re-create the issue on a fresh simulator running iOS 13.3 after additional testing.

Probably not worth any more of your time if you're ok with iOS 13+.

I will continue using my hack workaround until iOS 12 drops off significantly.

louisameline commented 4 years ago

Hi guys, I don't know if it's related, but take a look at this: https://github.com/quasarframework/quasar/issues/6695

It might be related, but not sure, because my own issue goes away with the Ionic webview, so...

rstoenescu commented 4 years ago

Yes, and we do highly recommend installing the ionic webview in docs..