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

iOS Companion App download broken on 3.x and 4.x release #808

Closed
irakhlin opened this issue Aug 17, 2022 · 12 comments
Closed

iOS Companion App download broken on 3.x and 4.x release #808

irakhlin opened this issue Aug 17, 2022 · 12 comments
Labels
bug Something isn't working

Comments

@irakhlin
Copy link

Release with the issue:
v3.0.0 and v4.0.0rc2
Last working release (if known):

Browser and Operating System:
Works on all browsers on iOS (safari, chrome and firefox). Does not work in the companion app, regardless of which browser is configured as the "launch with" in the settings.

Description of problem:
Clicking the download button for an event in the companion app simply has no effect, nothing is triggered. Interestingly clicking the "frigate" button works just fine and launches in the configured browser. Looking at the code the only difference I am seeing is that because you are calling homeAssistantSignPath the open.window call is being done in an async function. I am not a javascript developer but this sees to cause issues on iOS.

@irakhlin irakhlin added the bug Something isn't working label Aug 17, 2022
@dermotduffy
Copy link
Owner

Thanks @irakhlin , this has come up before a few times (e.g. #256 (comment) ) and no-one seems to have found a way to get the iOS companion app to support a download. Furthermore, I have no way to test this myself (neither real nor virtual that supports the companion app). Have you seen any other card that downloads correctly in the companion app in iOS, or have you ever seen this work in this card?

Looking at the code the only difference I am seeing is that because you are calling homeAssistantSignPath the open.window call is being done in an async function. I am not a javascript developer but this sees to cause issues on iOS.

That call is used for many other things (e.g. JSMPEG live views), so I suspect it's unlikely the issue. My current theory is just that the HA iOS companion simply won't let the user download anything somehow...

Back in 2021, you filed this comment which appears to suggest you did get this to work -- but perhaps that is just in the browser and not the companion app?

@irakhlin
Copy link
Author

irakhlin commented Aug 17, 2022

@dermotduffy

Interestingly it did previously work around that commit. It is not a feature I used often but my wife does and just recently pointed this out to me. I played around with the code and was able to get it to work but it is very hackish and may not be the best. One thing is that I have frigate integration enabled with "Enable the unauthenticated notification event proxy".

In card.ts; case 'frigate_ui' already worked for me in the iOS app, there you make a direct call to window.open and a frigate opens perfectly fine for me by launching the browser. The download case however does not, my thought here is that the reason for this is because window.open is being called in downloadViewerMedia() which is an async function which cannot be awaited there. I understand that this is being done so that you can call homeAssistantSignPath. Changing this around to look like this did seem to work https://github.com/irakhlin/frigate-hass-card/blob/713ba3100ff17a2433c444af89e74ca7296a7e3c/src/card.ts#L1202

I added a none async function just as a test called _downloadViewerMediaNew this means I was not able to call homeAssistantSignPath but instead generate the URL as is. This works just fine in the iOS companion app and when clicked the button opens a web browser and does indeed initiate a download of the file in question. I am guessing this only works because of enabling unauthenticated notification event proxy? I did find a few other instances of people having issues specifically in iOS calling window.open inside an async function:
https://stackoverflow.com/questions/70240682/window-open-with-ios-for-an-async-function-not-working
https://stackoverflow.com/questions/19026162/javascript-window-open-from-callback

Some of the solutions people present don't seem like they would work for this use case.

@dermotduffy
Copy link
Owner

@irakhlin Would you mind trying the code in #811 ?

It avoids opening the new window in the async context, and I think may help (although I have no way to test).

@irakhlin
Copy link
Author

@dermotduffy
Unfortunately this is not working. I tested on the newest iOS companion app along with the android companion app and neither work. My previous workaround does seem to work on both but again that is without using signurl at all. Do you have any ideas on how I could provide meaningful debug data?

@dermotduffy
Copy link
Owner

I tested on the newest iOS companion app along with the android companion app

I've tested this on multiple Android companion app instances and it works fine. May I ask how you're testing? Is it possible your cache isn't cleared?

My previous workaround does seem to work on both but again that is without using signurl at all.

Which makes it a non-starter I'm afraid since some users will require signing. It could be behind a configuration flag though, but I'd really like to get it working without needing to do that.

@irakhlin
Copy link
Author

I tested with the android beta-2606-e7dxdacd-full build which is the newest beta pushed from the google play store. For iOS it’s the newest none beta. When I test I installed the built add on in place of my current one, restart HA and clear the app cache. I used the debug button to verify the version that is loaded is from the ios branch. The tablet is an android galaxy tab a8 and the iphone is an iphone 11 with the newest ios. I will give this another shot tonight on box and see if there are any changes.

@dermotduffy
Copy link
Owner

When I test I installed the built add on in place of my current one

i.e. you do a build yourself from the PR branch and swap the output file in to replace the frigate-hass-card.js from your stock installation, then reset the app cache. If that's what you mean, I agree this should work.

used the debug button to verify the version that is loaded is from the ios branch

No idea why this wouldn't be working for you in Android (as at least we can share testing there). You could mind holding down the Frigate menu button and paste the full diagnostics information. That'll let me test exactly on an Android device and see if I can replicate your experience.

@irakhlin
Copy link
Author

irakhlin commented Aug 22, 2022

@dermotduffy

I just tested and you are correct it does indeed work on android. I did not realize the files were being downloaded as there was no prompt but it is working correctly. On iOS however I am seeing the same behavior, no prompt but also no file download; I tried it will safari, firefox and chrome set as the browser inside the home assistant app. I don't know if the debug information will be helpful for you on iOS but here it is.


{
  "ha_version": "2022.8.6",
  "card_version": "4.0.0-rc.2",
  "browser": "Mozilla/5.0 (iPhone; CPU iPhone OS 15_6_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Home Assistant/2022.3 (io.robbie.HomeAssistant; build:2022.358; iOS 15.6.1) Mobile/HomeAssistant, like Safari",
  "date": "2022-08-22T08:53:17.342Z",
  "frigate_version": {
    "ec2c8925d22540a93b09b04b2678f925": "3.0.0-rc.3/0.11.0-ef54cd6"
  },
  "lang": "en",
  "git": {
    "build_version": "4.0.0-rc.2-ios-download+g1d7f916",
    "build_date": "Mon, 22 Aug 2022 08:32:28 GMT",
    "commit_date": "Fri, 19 Aug 2022 19:37:10 -0700"
  },
  "config": {
    "type": "custom:frigate-card",
    "view": {
      "default": "live"
    },
    "live": {
      "preload": true,
      "webrtc_card": {
        "background": false,
        "ice_servers": [
          {
            "urls": "removed ",
            "username": "removed",
            "credential": "removed"
          },
          {
            "urls": "turn:removed",
            "username": "removed",
            "credential": "removed"
          }
        ]
      },
      "lazy_load": true,
      "lazy_unload": "all"
    },
    "menu": {
      "buttons": {
        "frigate": {
          "enabled": true
        },
        "image": {
          "enabled": false
        },
        "download": {
          "enabled": true
        }
      },
      "style": "outside",
      "position": "bottom"
    },
    "dimensions": {
      "aspect_ratio_mode": "dynamic"
    },
    "cameras": [
      {
        "camera_entity": "camera.back_2",
        "live_provider": "webrtc-card",
        "webrtc_card": {
          "entity": "camera.shed_camera_mediaprofile_channel1_substream2"
        },
        "frigate": {
          "camera_name": "back"
        }
      }
    ],
    "media_viewer": {
      "controls": {
        "next_previous": {
          "style": "chevrons"
        }
      }
    }
  }
}

edit: I also found this that may be helpful, https://www.reddit.com/r/webdev/comments/hjupnf/safari_ios_on_mobile_not_opening_a_new_window/?utm_source=share&utm_medium=ios_app&utm_name=iossmf

TL;DR Safari mobile will not let you call window.open() within nested functions. They must be on the event listener itself.

@dermotduffy
Copy link
Owner

I just tested and you are correct it does indeed work on android. I did not realize the files were being downloaded as there was no prompt but it is working correctly.

OK, great.

TL;DR Safari mobile will not let you call window.open() within nested functions. They must be on the event listener itself.

If that's the case then I don't understand how your earlier modification worked -- since it's certainly not calling window.open() in the direct event listener function...

@dermotduffy
Copy link
Owner

TL;DR Safari mobile will not let you call window.open() within nested functions. They must be on the event listener itself.

If that's the case then I don't understand how your earlier modification worked -- since it's certainly not calling window.open() in the direct event listener function...

@irakhlin Any thoughts on the above? If the requirement is really as you have expressed, I suspect I'll close this bug and simply not support downloads on the app in iOS (as it's a completely unreasonable limitation IMO).

But really I'm lacking a way to test, and thus a way to make progress. Maybe I can setup some emulation, but this will not be high on my TODO list realistically.

@irakhlin
Copy link
Author

TL;DR Safari mobile will not let you call window.open() within nested functions. They must be on the event listener itself.

If that's the case then I don't understand how your earlier modification worked -- since it's certainly not calling window.open() in the direct event listener function...

@irakhlin Any thoughts on the above? If the requirement is really as you have expressed, I suspect I'll close this bug and simply not support downloads on the app in iOS (as it's a completely unreasonable limitation IMO).

But really I'm lacking a way to test, and thus a way to make progress. Maybe I can setup some emulation, but this will not be high on my TODO list realistically.

@dermotduffy I wouldn't blame you for closing this but it is a little strange that this somehow works with my obviously crude and obviously not release quality modification. Sorry I have not looked into this more as I was a bit busy last couple weeks but if its of any help I would be more than happy to test for you, I looked into emulation of running OS X on vmware and while it works .. its incredibly slow. I would be testing on real iPhones. Just let me know what specific information would be of use in terms of debugging.

@dermotduffy dermotduffy added this to the v4.1 milestone Mar 1, 2023
@dermotduffy
Copy link
Owner

I am closing this since (1) I have no way to test this, (2) It works fine for all browsers, including browsers on iOS, (3) it seems like it might be an issue with the iOS app itself. See related:

home-assistant/iOS#2283

That user reports the same the issue both in the card and the addon, suggesting the issue is not with the card. Lets follow-on on that issue, and if there's some workaround identified I will reopen & implement here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants