Moonshine-IDE / Super.Human.Portal

Portal interface to show documentation for DominoVagrant and Super.Human.Installer
Other
0 stars 1 forks source link

Open Nomad links in same Window #53

Closed piotrzarzycki21 closed 4 months ago

piotrzarzycki21 commented 6 months ago

From HCL:

Greetings Justin and the Prominic team,

Thank you for meeting with us to discuss the Nomad team's progress regarding a number of issues. I have attached the slide deck shared in the 20220222 checkpoint meeting detailing the launch handler and link capturing feature experiments in Chrome and Nomad Web. Since we last spoke, the Chromium team has communicated that this experimental feature will not be available in Chrome release 123 due to issues they're seeing with the current implementation. We will continue to monitor and test their changes accordingly in order to improve the link handling experience in Nomad.

On slides 3-5, I've added a number of scenarios and the observed behaviors that an end user may experience. Each of these are under the condition that both the Chrome experiment and Nomad experiment flags are enabled.

We also discussed an alternative solution that may suit Prominic's needs by using service worker messaging to open links in an existing Nomad browser tab or PWA window. Slide 6 is an explanation of how Nomad Web is using this messaging API to trigger the opening of links in the main Nomad window if the browser has opened an intermediate secondary window. On slides 7-9, I have included an example web page, scopedlinks.html, that leverages that functionality by overriding the browser's default behavior of clicking on a link to use the navigator.serviceWorker.controller.postMessage() API instead. You may edit and test this file with links appropriate to your deployment. Paste this file in the nomad-files/ directory with the rest of the Nomad Web client files. This will ensure that the postMessage() calls are messaging Nomad's service worker. Note that this is not using launch handler or link capturing features. The experiments do not need to be Enabled to test this page.

Thank you for your time to continue to discuss these issues as we work toward a solution. Please feel free to reach out if you have any further questions.

scopedlinks.html.zip

Summary from @piotrzarzycki21

Here is the context of our short chat today. They provided example in html file, I have added that code to Super.Human.Portal in Browse My Server - select some application and click "Open in Nomad" their code will be executed. Lates build should have my changes. If you open Web Developer tools and go to file MediatorBrowseMyServer jump to method: onOpenNomadWeb - place there debugging break point and you will be able see that ServiceWorker is in pending state and controller is null.

I'm wondering based on their description below whether we need some special deployment for Super.Human.Portal to have it working. Maybe you will have some thoughts on that - if not let me know and I will respond to this email asking for assistance.

UPDATE: Branch: https://github.com/Moonshine-IDE/Super.Human.Portal/tree/features/issue_53_nomad_links

JoelProminic commented 5 months ago

@piotrzarzycki21, I spent a little time checking if serviceWorker.controller.postMessage reports an error somehow.

The most promising option I found was the error event on the serviceWorker.controller object.

It looks like our best bet would be to check the message event on the serviceWorker object event, but these won't necessarily all be errors. I do see an error event as well, but it is deprecated.

JoelProminic commented 5 months ago

Testing with today's updates, I nomadhelper.html is not working at all for me. Even if Nomad is open, the database is not opened in the Nomad tab. Testing with the debugger, I see that the snackbar message is sent before the postMessage is even called.

UPDATE: I found that nomadhelper.html was out of date on my test machine, but I am still seeing some problems. The links are opening in the nomad tabs in some cases, but it seems inconsistent or unusually slow. I am testing in macOS Firefox.

piotrzarzycki21 commented 5 months ago

UPDATE: I found that nomadhelper.html was out of date on my test machine, but I am still seeing some problems. The links are opening in the nomad tabs in some cases, but it seems inconsistent or unusually slow. I am testing in macOS Firefox.

@piotrzarzycki21 Unfortunately I fount that as well inconsistent. It really depends on what kind of database we are trying to open - at least I think so. Additionally that this slowness maybe because of your heavy usage of Firefox. To me Firefox is not main browser and everything is working quite fast always.

I just pushed changes to Royale code and nomadhelper.html where message is being displayed once nomadhelper.html report Success after calling openNomadUri.

piotrzarzycki21 commented 5 months ago

@JoelProminic I have added to nomadhelper.html following code. Unfortunately when I compared objects which I'm getting in events between Opened vs Closed Nomad tab - there is no clue inside that objects whether Nomad Tab is opened. Events objects looks the same in both cases. Try yourself by adding that code and deploying it in newest nomadhelper.html.

if (navigator && navigator.serviceWorker && navigator.serviceWorker.controller) {
                navigator.serviceWorker.oncontrollerchange = function(event) {
                    console.log('ServiceWorker: [oncontrollerchange]');
                        console.log(event);
                }
                navigator.serviceWorker.onmessage = function(event) {
                        console.log('ServiceWorker: [onmessage]');
                        console.log(event);
                }
                 navigator.serviceWorker.onmessageerror = function(event) {
                    console.log('ServiceWorker: [onmessageerror]');
                        console.log(event);
                }

                navigator.serviceWorker.controller.onstatechange = function(event) {
                    console.log('Controller: [onstatechange]');
                        console.log(event);
                }

                navigator.serviceWorker.controller.onerror = function(event) {
                    console.log('Controller: [onerror]');
                        console.log(event);
                }

            navigator.serviceWorker.controller.postMessage({
                type: 'openNotesUri',
                payload: {
                    notesUri: decodeURIComponent(
                        hrefUrl.hash.substring(
                            hrefUrl.hash.indexOf(HASH_PREFIX) + HASH_PREFIX.length
                        )
                    )
                }
            });
            console.log(navigator.serviceWorker);
            console.log(navigator.serviceWorker.controller);
            window.top.postMessage('[Success] Successfully called openNotesUri', '*');
        } else {
            console.error('No service worker registered');

                window.top.postMessage('[Error] No service worker registered', '*');
        }
piotrzarzycki21 commented 5 months ago

@JoelProminic branch has been merged to main.

JoelProminic commented 5 months ago

Suggestions from Daniel from HCL

The scopedlinks.html page is itself a client to the Nomad service worker. If itself and Nomad are opened, you'll see the two clients listed in Chrome Dev Tools:

If Nomad was closed, but scopedlinks.html is still open, just the one client is listed:

Any checks that that page performs against navigator.serviceWorker.controller will be a non-null value, assuming the service worker is installed.

I believe what could help with your scenario is instead of checking navigator.serviceWorker.controller, you can check the client count. This is queried from the service worker as it's a service-worker only API ( https://developer.mozilla.org/en-US/docs/Web/API/Clients ). If the Nomad service worker finds more than one window client opened, then we can assume that a Nomad window is also opened, and you can continue with the postMessage to open the link.

First you'll need to register a service worker message listener in scopedlinks.html ( https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorkerContainer/message_event ). Listen for the event.data.type 'numberOfWindowClients'. When the service worker posts a message payload back to the client, you can get the number of window clients in this message handler via event.data.clientCount.

Then, to query the service worker for the client count, perform a postMessage to the service worker ( https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorker/postMessage ) with message { type: 'getNumberOfWindowClients' }

Note that this is an asynchronous operation because it's message-driven. You'll have to wait for the client count response before moving forward with the rest of the logic.

piotrzarzycki21 commented 5 months ago

Then, to query the service worker for the client count, perform a postMessage to the service worker ( https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorker/postMessage ) with message { type: 'getNumberOfWindowClients' }

Unfortunately whatever I tried this part doesn't work. I will share details during our today's meeting.

JoelProminic commented 5 months ago

From our testing today, it looks like the postMessage should be numberOfWindowClients instead of getNumberOfWindowClients.

The flow would be:

  1. Super.Human.Portal calls nomadhelper.html with a URL
  2. nomadhelper.html sets up the message handlers
  3. nomadhelper.html sends a postMessage with numberOfWindowClients
  4. message event triggers with type:numberOfWindowClients
  5. Check the count
    1. If it indicates that there is an active client, submit the openNotesUri postMessage as before
    2. If it indicates that there is not an active client, report an error back to Super.Human.Portal
JoelProminic commented 5 months ago

I did some digging in the service worker from the Firefox debug console, and I parsed a list of likely commands from /nomad/sw-bundle.js. If you see something that may be useful, you can dig through the JavaScript to try and figure out what it does, but the code is minimized.

authenticate
authenticated
broadcastHideBackdrop
broadcastNomadUpdating
broadcastResetNomad
broadcastShowBackdrop
claimClients
clientClosing
createIFrame
end
focusPrimaryWindow
fromWaitingSW-returnNewAndCurrentVersion
getSWVersion
newClient
numberOfWindowClients
openNotesUri
skipWaiting
startDownload
toWaitingSW-returnNewAndCurrentVersion
updateOnlineOffline
JoelProminic commented 4 months ago

Testing the latest nomadhelper.html and Super.Human.Portal on the Prominic servers, I noticed that nomadhelper.html seems to keep triggering the URLs. I saw the database open multiple times, and they would reopn after I closed them. The Prominic Nomad server was running Nomad "1.0.10.5955-3678"

I'll do some more tests to see if this triggers on SHI servers as well.

JoelProminic commented 4 months ago

UPDATE: I confirmed this with a Super.Human.Installer instance (Nomad 1.0.10).

  1. Close Nomad and reload Super.Human.Portal
  2. Click an Open in Nomad button. BUG: This is getting blocked as a popup with the latest build - this wasn't triggering in my other recent tests. I allowed the individual popup to continue. See example below
  3. Nomad opens in a new tab
  4. Click an Open in Nomad button.
  5. The database opens in the existing Nomad tab
  6. Close the database tab
  7. BUG: The database reopens after a short delay.

image

piotrzarzycki21 commented 4 months ago
  • Close Nomad and reload Super.Human.Portal
  • Click an Open in Nomad button. BUG: This is getting blocked as a popup with the latest build - this wasn't triggering in my other recent tests. I allowed the individual popup to continue. See example below

@JoelProminic I have recorded video - it looks like on my sight it doesn't trigger or I don't understand what do you mean by that.

https://github.com/Moonshine-IDE/Super.Human.Portal/assets/24554795/3a813068-30b5-4d60-9f11-7894bd40c4d2

piotrzarzycki21 commented 4 months ago

7. BUG: The database reopens after a short delay.

This part I have reproduced. In newest build it's fixed.

JoelProminic commented 4 months ago

I'm still getting the popups on domino-49.prominic.net. Check your popup settings and exclusions to see if you excluded the test server previously. For Firefox, this is FireFox > Settings > Privacy & Security > Block pop-up windows > Exceptions.

piotrzarzycki21 commented 4 months ago

What about Bug number 7?

JoelProminic commented 4 months ago

In addition to the popup bar, I get this message on the console:

Uncaught Error: navigateToURL requires interaction trigger
    navigateToURL https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:41083
    controller_CommandLaunchNomadLink_onWindowMessage https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:37030
    execute https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:37001
    executeCommand https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:53191
    notifyObserver https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:36861
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:19369
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43936
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43932
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12519
    mediator_applications_MediatorInstalledApps_onOpenInNomadLink https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42557
    fireListeners https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12857
    dispatchEventInternal_ https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9429
    dispatchEvent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12837
    dispatchEvent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12984
    view_controls_LinkWithDescriptionAppButton_onLinkClick https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:55878
    $EH_13_0 https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:55903
    fireListener https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9215
    fireListenerOverride https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12903
    handleBrowserEvent_ https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9259
    f https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9021
    addEventListener eval:4
    listen_ https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9007
    listen https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:8970
    addEventListener https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12948
    initializeStrandBasedObject https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57650
    generateMXMLArray https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57552
    generateMXMLInstances https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57676
    addedToParent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14510
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:13597
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14395
    initializeStrandBasedObject https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57665
    generateMXMLArray https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57552
    generateMXMLInstances https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57676
    addedToParent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14510
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:13597
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14395
    mediator_applications_MediatorInstalledApps_updateInstalledAppLinks https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42503
    mediator_applications_MediatorInstalledApps_updateView https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42458
    onRegister https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42433
    registerMediator https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:19398
    registerMediator https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43913
    execute https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:22738
    executeCommand https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:53191
    notifyObserver https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:36861
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:19369
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43936
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43932
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12519
    mediator_MediatorMainContentView_onNavigationInstalledAppSectionChange https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:36383
    fireListener https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9215
SuperHumanPortal_Royale.js:41083:11

The database reopening issue is resolved in my quick test. I'll report if I start seeing it again.

UPDATE: I'm not sure what you are looking for with "Bug number". This is the same issue I reported for step 2 in my last test procedure:

  1. Click an Open in Nomad button. BUG: This is getting blocked as a popup with the latest build - this wasn't triggering in my other recent tests. I allowed the individual popup to continue. See example below
piotrzarzycki21 commented 4 months ago

In addition to the popup bar, I get this message on the console:

Uncaught Error: navigateToURL requires interaction trigger
    navigateToURL https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:41083
    controller_CommandLaunchNomadLink_onWindowMessage https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:37030
    execute https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:37001
    executeCommand https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:53191
    notifyObserver https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:36861
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:19369
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43936
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43932
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12519
    mediator_applications_MediatorInstalledApps_onOpenInNomadLink https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42557
    fireListeners https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12857
    dispatchEventInternal_ https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9429
    dispatchEvent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12837
    dispatchEvent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12984
    view_controls_LinkWithDescriptionAppButton_onLinkClick https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:55878
    $EH_13_0 https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:55903
    fireListener https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9215
    fireListenerOverride https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12903
    handleBrowserEvent_ https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9259
    f https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9021
    addEventListener eval:4
    listen_ https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9007
    listen https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:8970
    addEventListener https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12948
    initializeStrandBasedObject https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57650
    generateMXMLArray https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57552
    generateMXMLInstances https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57676
    addedToParent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14510
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:13597
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14395
    initializeStrandBasedObject https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57665
    generateMXMLArray https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57552
    generateMXMLInstances https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:57676
    addedToParent https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14510
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:13597
    addElement https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:14395
    mediator_applications_MediatorInstalledApps_updateInstalledAppLinks https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42503
    mediator_applications_MediatorInstalledApps_updateView https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42458
    onRegister https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:42433
    registerMediator https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:19398
    registerMediator https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43913
    execute https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:22738
    executeCommand https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:53191
    notifyObserver https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:36861
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:19369
    notifyObservers https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43936
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:43932
    sendNotification https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:12519
    mediator_MediatorMainContentView_onNavigationInstalledAppSectionChange https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:36383
    fireListener https://<-snip->/Super.Human.Portal/js-debug/SuperHumanPortal_Royale.js:9215
SuperHumanPortal_Royale.js:41083:11

The database reopening issue is resolved in my quick test. I'll report if I start seeing it again.

UPDATE: I'm not sure what you are looking for with "Bug number". This is the same issue I reported for step 2 in my last test procedure:

  1. Click an Open in Nomad button. BUG: This is getting blocked as a popup with the latest build - this wasn't triggering in my other recent tests. I allowed the individual popup to continue. See example below

I have cleaned up above exception - it occurs when browser blocking popups window.open returns null in that case and internal logic of navigateToURL throwing exeption.

piotrzarzycki21 commented 4 months ago

2. Click an Open in Nomad button. BUG: This is getting blocked as a popup with the latest build - this wasn't triggering in my other recent tests. I allowed the individual popup to continue. See example below

@JoelProminic I'm not sure what is the expectation here. I'm able to reproduce scenario where browser is blocking my popup by removing it from Block pop-up windows > Exceptions. However I don't think it is my responsibility to do something more for the user. I do see space for displaying Snackbar message when popup is blocked because navigateToURL throwing exception. Do you want me to do this ?

JoelProminic commented 4 months ago

@piotrzarzycki21, I wasn't seeing the Nomad links blocked by the popup blocker until the changes this week, so I'd like to approach this by figuring out what has changed. Does this happen because we are using navigateToURL different, or trying to open the links from nomadhelper.html?

I feel the (blocked) popups are going to create a bad user experience. Even if this will be primarily be used by the Prominic support team, there are some users who will immediately lose respect for the application if they have too mess with popups.

If you don't see any good leads for this, we can discuss it more in the meeting on Tuesday.

JoelProminic commented 4 months ago

To clarify:

  1. Clear nomad_helper_url in SuperHumanPortal.nsf
  2. Refresh Super.Human.Portal
  3. Click a button to trigger the Open in Nomad logic
  4. Opens a new tab. This is not blocked by the popup blocker
  5. Set nomad_helper_url
  6. Close Nomad tab
  7. Refresh Super.Human.Portal
  8. Click a button to trigger the Open in Nomad logic
  9. BUG: triggers popup blocker
  10. Allow this popup. It opens a new tab which launches Nomad.
piotrzarzycki21 commented 4 months ago

I believe there is some ideas how to avoid blockers here. I just need to experiment to do this. I'm not sure if this is suppose to be a blocker.

JoelProminic commented 4 months ago

The first answer to that question is my understanding of the browser popup-blocker behavior. The browser is not able to trace the button click through to the window open in this case, so it blocks the action.

The second action looks promising, though I'm not clear on how it will look to the user. If it is opening and closing the new tab and changing the user focus, that could be distracting.

I feel that this is a blocker, since it seems like the kind of behavior that support would need to guide non-technical users through, and that technical users would find suspicious.. I remember @JustinProminic also had concerns for this, but he may decide to bump this to the next release if he wants to get this release fully pushed out.

JoelProminic commented 4 months ago

Here is a new issue I found while testing on the Prominic server. nomadhelper.html got an error message indicating that the Nomad window had closed, but it did not attempt to open a new tab:

Nomad Window is closed nomadhelper.html:39:15
    messageFunction https://-snip-:9443/nomad/nomadhelper.html?link=https://-snip-:9443/nomad/#/notes://-snip-?OpenView:39
    (Async: EventListener.handleEvent)
    openLink https://-snip-:9443/nomad/nomadhelper.html?link=https://-snip-:9443/nomad/#/notes://-snip-?OpenView:47
    onload https://-snip-:9443/nomad/nomadhelper.html?link=https://-snip-:9443/nomad/#/notes://-snip-?OpenView:12
    (Async: EventHandlerNonNull)
    <anonymous> https://-snip-:9443/nomad/nomadhelper.html?link=https://-snip-:9443/nomad/#/notes://-snip-?OpenView:9
JoelProminic commented 4 months ago

@JustinProminic did tests with today, and noticed these problems:

I confirmed that nomadhelper.html was up-to-date on the Nomad server.

Justin suggested that we show a message telling the user how to get past the popups. Ideally this would have arrows showing users where to click.

@MarkProminic found this option for changing the browser popup behavior. This has limited browser support, though.

JoelProminic commented 4 months ago

Unfortunately, it looks to me like browserSettings.allowPopupsForUserEvents is a feature for browser extensions rather than for general JavaScript. This also looks to be a global setting, so it wouldn't make sense to allow unrestricted access to websites.

I think the solution Piotr found is the best option for resolving the popup blocker events. If this doesn't work, then we may need to switch over to @JustinProminic's design.

piotrzarzycki21 commented 4 months ago

5. While the bookmark is collapsed, click the bookmark. BUG: Nothing happens. This is expected to trigger the Open in Nomad logic by default

@JoelProminic I have fixed this bug. It's in newest build.

piotrzarzycki21 commented 4 months ago

The first answer to that question is my understanding of the browser popup-blocker behavior. The browser is not able to trace the button click through to the window open in this case, so it blocks the action.

The second action looks promising, though I'm not clear on how it will look to the user. If it is opening and closing the new tab and changing the user focus, that could be distracting.

I feel that this is a blocker, since it seems like the kind of behavior that support would need to guide non-technical users through, and that technical users would find suspicious.. I remember @JustinProminic also had concerns for this, but he may decide to bump this to the next release if he wants to get this release fully pushed out.

@JoelProminic proposed solution with opening empty window and replace url from SOW didn't work for me. I've decided to inform use what to do when popup is blocked. I'm displaying following message. We should discuss if this is acceptable solution tomorrow. My changes are pushed so you can test it.

Screenshot 2024-05-27 at 20 20 27
piotrzarzycki21 commented 4 months ago

@JoelProminic I think I have fixed this NPE which we have experienced during meeting. Newest build should have my changes.

JoelProminic commented 4 months ago

I confirmed that the issue when switching categories under Bookmarks was resolved.

There is a remaining issue where a database opens twice when the Nomad window was not open, but this seems to be an issue on the Nomad side.