standardnotes / forum

Support from other community members. For 1-on-1 help, please contact help@standardnotes.com.
https://forum.standardnotes.org
196 stars 9 forks source link

AutoLock not locking in Firefox on window blur #1956

Closed parallelo3301 closed 4 years ago

parallelo3301 commented 4 years ago

https://github.com/standardnotes/web/blob/ec0555ad2646eb6372c0cde273fed0c7d0ed19a8/app/assets/javascripts/app/services/passcodeManager.js#L177-L180

Problem: Relying on the visibilitychange for tracking the page visibility is not working in Mozilla Firefox as the event is not being triggered on program/window change. Event is only triggered on the tab change. Works in Chrome without any problem.

Solution: Listening for the blur & focus (for clearing the timer) events should work.

Demo: https://jsbin.com/havewasibe Source: https://jsbin.com/havewasibe/edit?html,js,console,output

How to simulate: Open dev tools & try changing the program/window. Then try to change the tabs also.

moughxyz commented 4 years ago

blur event isn't getting called for me on either Firefox or Chrome (latest) on macOS. I believe tab change was always the intended behavior.

parallelo3301 commented 4 years ago

Forgot to write that I was testing on Windows.

blur event isn't getting called for me on either Firefox or Chrome (latest) on macOS.

Finally found some Mac to test it and blur is called without any issue for me (Safari & Firefox). I thought that using only the blur & focus events would be enough, but visibilitychange is needed for tracking the tab change also on Mac (as blur is not being called on the tab change on Mac, but IS being called on Windows).

I believe tab change was always the intended behavior.

It doesn't make sense to lock only on tab change. AutoLock is also described as The autolock timer begins when the window or tab loses focus., so yes, tab change is intended, but not locking on window change is a bug.

See also: https://www.quirksmode.org/dom/events/blurfocus.html

BTW: Thanks for the project, I finally found the best solution for my knoledge base! I'm thinking about writing some editors (like draw.io integration if possible for diagrams & mind maps) & WebSSH client (the note will contain the config, private key, etc.) if I find any spare time to do it.

moughxyz commented 4 years ago

The demo still doesn't log 'blur' when I blur the window, in either Chrome, Firefox, or Safari. Could you update the demo to get this working?

parallelo3301 commented 4 years ago

I used the same demo and it worked. I don't have available Mac, so can you try this one? https://jsbin.com/lamadekido/edit?html,js,console,output? I've added focus, pagehide & pageshow events (found there https://gist.github.com/gertjanmeire/7158190#file-js-detect-browser-js-L4-L6).

moughxyz commented 4 years ago

Nope, no events triggered when I select another window on macOS 10.14.6 (using Chrome 78.0.3904.97).

now-raymond commented 4 years ago

@fandaa's latest demo works fine for me with Chrome on macOS 10.15.1.

@mobitar Have you clicked into the output iframe to give it focus? JSBin keeps its output in a sandbox iframe, so the top-level event listeners won't fire while the output window is not in focus.

moughxyz commented 4 years ago

Ah yeah, that was it. Needed to click on the iframe first, then click away.

parallelo3301 commented 4 years ago

@now-raymond thanks for testing & the testing tip.

@mobitar Sorry, I didn't realized that as I was clicking in/outside that can be a problem. I previously linked demo, but then found out that it's only available for 30 minutes or something like that, then it takes you directly to the editor. It would work normally in output & devtools. :-/

As I said before, listening for the blur & focus events should be added, as they are not fired on tab change, so Page Visibility events are still needed also.

bryvin commented 4 years ago

I just tried to implement a window blur/focus event combo to trigger the Passcode Lock. Unfortunately since Standard Notes also uses iframes similar to JSBin where they are sandboxed, I am fairly certain it prevents blur/focus from being a viable solution.

Anytime I would click into an extended editor or even try to use the Extensions panel and do any action within (click on any of the buttons to install/un-install/view info/etc) it would trigger the lock.

The original event would trigger off the main window and once you were active on the next iframe (editor, extension panel) if you tried to cause the next blur event by leaving the Browser window it would not trigger the event again as the 'window' of the document was technically already blurred.

Lastly, on FireFox even just loading an extended editor by switching notes would trigger the lock and when you unlocked it would be back at the note you we previously on.. essentially making it so you couldn't access anything other than your current note until you turned off the Passcode.

now-raymond commented 4 years ago

Indeed blur and focus become much less straightforward when iframes are involved since they steal focus from the top-level window.

document.hasFocus() can tell you whether any element inside the document has focus and, at least on Chrome, works for iframes. It also works for tab changes and active window changes.

MDN has a live sample here: https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus

Unfortunately document.hasFocus() is a one-off function, not an event.

If users aren't expected to spend the majority of their time in an iframe, perhaps a hybrid solution could work, where a timer gets set up to poll focus only while the user is interacting within an iframe, and the event-based approach handles interactions otherwise.

In the window-level onblur event handler, checking document.hasFocus() could tell you whether focus was truly lost or was transferred into an iframe.

bryvin commented 4 years ago

@now-raymond Thanks for the detailed information. I will take another look at this using document.hasFocus() !

bryvin commented 4 years ago

I've committed the addition of verifying document.hasFocus() and triggering the Autolock if it loses focus. This change should be pushed live soon and there will be further improvements in the future; as you described to only check for focus if a blur event has previously been triggered but that will require some additional testing and I wanted to at least get a patch out to ensure the app locks when it should.

Thanks for the help and information!