Strangeness under google-chrome on linux (edge/brave on windows)

[rouilj]

I have been testing with firefox. The test suite is working with firefox (except for a title n window issue).

However in google chrome, the popup window is displayed, but it is empty and it has no title.
No errors are reported in the console.

This is with Version 124.0.6367.118 (Official Build) (64-bit) on linux, Very weird as I am sure it was working
in the past.

Can anybody duplicate this?

[rouilj]

Hmm, it looks like a git pull messed up somehow.

This may be nothing.

[rouilj]

Ok, I have synced to b74ac89 and I still see an issue.

The error is:

TypeError: Cannot read properties of null (reading 'addEventListener')
at ClassHelper.openPopUp (classhelper.js:897:23)

and is thrown in this code:

his.openPopUp(initialRequestURL, this.helpurlProps)
                .catch(error => {
                    // Top level error handling for openPopUp method.
                    cleanUpClosure();
                    console.error(error);
                    if (this.popupRef != null) {
                        this.popupRef.close();
                    }
                    window.alert("Error: Failed to open classhelper, check console for more details.");
                    this.helpurl.click();
                });

[rouilj]

Ti looks like

this.popupRef = window.open(CLASSHELPER_POPUP_URL, CLASSHELPER_POPUP_TARGET, popupFeatures);

window.open is returning null. I don't think I have popup blocking turned on.

[rouilj]

Malav can you take a look at this when you get a chance. Even when the fallback occurs, the popup opens in the main window not as a popup.

I have allowed popup windows in the browser.

[malvpatel]

@rouilj I am not able to reproduce this issue

Chrome - Version 124.0.6367.118 (Official Build) (64-bit)
at b74ac89

Is this with front end testing? or just the regular window

[malvpatel]

@rouilj The best we can do is add a null check as window.open(...) can return a null

        if (this.popupRef == null) {
            throw new Error("Browser Failed to open Popup Window");
        }

[rouilj]

Hi Malav: In message ***@***.***>, Malav Patel writes:

@rouilj The best we can do is add a null check as `window.open(...)` can return a null ``` if (this.popupRef == null) { throw new Error("Browser Failed to open Popup Window"); } ```

Yes, let's do that and gather as much info as we can as to why it failed. I can reproduce the failure in my Brave browser on windows, but not in my chrome browser on windows. I have a couple more browsers I can try to narrow this down before I get out the debugger again.

…

-- -- rouilj John Rouillard =========================================================================== My employers don't acknowledge my existence much less my opinions.

[rouilj]

Also when the fallback triggers, I see the code trying to reset the onclick property. But it's not working right.
The click you trigger causes the popup to open in the same window replacing the issue item display.
This patch works to open the popup in a popup window:

diff --git a/html/classhelper.js b/html/classhelper.js
index 33ef4384..f9c3cd39 100644
--- a/html/classhelper.js
+++ b/html/classhelper.js
@@ -132,7 +132,7 @@ class ClassHelper extends HTMLElement {
             console.warn("Classhelper not intercepting helpurl.");
             if (this.helpurl != null) {
                 this.helpurl.removeEventListener("click", this.preventDefault);
-                this.helpurl.setAttribute("onclick", this.helpurlScript);
+                this.helpurl.onclick = this.helpurlScript);
             }
             console.error(err);
             return;
@@ -156,7 +156,7 @@ class ClassHelper extends HTMLElement {
             console.warn("Classhelper not intercepting helpurl.");
             this.removeEventListener("click", handleClickEvent);
             this.helpurl.removeEventListener("click", this.preventDefault);
-            this.helpurl.setAttribute("onclick", this.helpurlScript);
+            this.helpurl.onclick = this.helpurlScript;
         }

         const handleClickEvent = (event) => {

I think the difference is I am setting the property onclick and not the attribute onclick. The attribute
IIUC is the serialized HTML version of the thing. Some attributes are synced to properties and some are not.

https://jakearchibald.com/2024/attributes-vs-properties/ makes it clearer but....

Still trying to track down where the failure is that's causing the web component to trigger
the fallback.

[rouilj]

Hmm, it looks like the load event for the window is never firing (or is sent before the eventhandler for load is established). So it's left empty.

I tested this by adding a breakpoint inside the load handler and it is never reached.

Maybe before you establish the eventhander for load, check to see if https://developer.mozilla.org/en-US/docs/Web/API/Document/readyState readyState is already complete and fire the code to
populate the popup?

Thoughts?

[rouilj]

I changed the code in openPopUp() to read:

        const popupFeatures = CLASSHELPER_POPUP_FEATURES(props.width, props.height);
        this.popupRef = window.open(CLASSHELPER_POPUP_URL, CLASSHELPER_POPUP_TARGET, popupFeatures);

        console.log(this.popupRef.document.readyState)

        this.popupRef.addEventListener("load", (event) => {
            const doc = event.target;
            const body = doc.body;

and console.log is showing complete every time and the window is empty in brave.

In windows chrome I see: interactive and in windows FF I see: uninitialized.
In edge I also see complete and the popup doesn't initialize.

So it looks like a lifecycle timing difference between browsers.
Fun.

[malvpatel]

Umm setTimeout() 3 secs ? Or I open popup before making the first api request Sent from Proton Mail Android

…

-------- Original Message --------

On 15/05/24 15:28, John P. Rouillard wrote: I changed the code in openPopUp() to read: const popupFeatures = CLASSHELPER_POPUP_FEATURES(props.width, props.height); this.popupRef = window.open(CLASSHELPER_POPUP_URL, CLASSHELPER_POPUP_TARGET, popupFeatures); console.log(this.popupRef.document.readyState) this.popupRef.addEventListener("load", (event) => { const doc = event.target; const body = doc.body; and console.log is showing complete every time and the window is empty in brave. In windows chrome I see: interactive and in windows FF I see: uninitialized. In edge I also see complete and the popup doesn't initialize. So it looks like a lifecycle timing difference between browsers. Fun. — Reply to this email directly, [view it on GitHub](#56 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AIMOJXUOBAKYJTYQJDUUMMLZCOZPJAVCNFSM6AAAAABHYNDGKWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJTGMYDSOJZGM). You are receiving this because you were assigned.Message ID: ***@***.***>

[rouilj]

I'm not sure what you want to trigger with the setTimeout(...,3000). I dislike (but have used) magic timing
numbers in general. Also waiting three seconds to populate the displayed popup window isn't great.

Would this work:

1 check the status of readyState
2 set up the load handler as you currently have it
3 set up the keyboard handler as you currently have it
4 if readyState was 'complete' do a setTimeout to send a 'load' event to the popup window.

for 4 something like:

const event = new Event("load");

// Dispatch the event.
setTimeout(this.popUpRef.dispatchEvent, 0, event)

This should trigger the load event handler on the window in the next interval (as soon as openPopUp
returns most likely).

It is possible you may get two load events. The load event generated by the browser could arrive after
the load event handler is established even if readyState == complete before the load handler is set up.
If that happens, the second load event sent by setTimeout arrives after the classhelper popup
has been populated. I think this second event can be ignored if the event handler check to see if doc.body is empty. If doc.body is empty just return from the load event handler assuming the load event handler has
already run.

I am assuming that the readyState won't change to "complete" while javascript to setup the event handlers
is executing since JS is single threaded. This might be incorrect as the popup window may have it's own
interpreter but... (can't really find anything that discusses how the lifecycle works between js in one
window an a window.open'ed window).

Thoughts?

Also since I was able to reproduce this with Edge, can @Nikunj-1308 or @BharathKanama se the
issue on their windows boxes?