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

nw2: tray icon from bg-script is detached from app after reloading the app #7223

Closed
Luzzifus opened this issue Nov 19, 2019 · 10 comments
Closed
Labels

Comments

@Luzzifus
Copy link

Luzzifus commented Nov 19, 2019

NWJS Version : 0.42.5
Operating System : Win10 (1903)

Expected behavior

Initializing a tray icon via bg-script, according to the tray doc page, the tray icon should still work after an app page reload via context menu or dev tools.

Actual behavior

Well, it does not. It still gets detached from the app after reloading and it also won't be destroyed when the app is closed.

How to reproduce

Only happens in NW2 mode, works as expected with "chromium-args": "--disable-features=nw2".

My tray script:

let requestAction = action => global.model.app.requestAction = action;
let trayOptions = { title: 'Test', tooltip: 'test', icon: 'html/icon/icon016.png' };
let tray = new nw.Tray(trayOptions);
tray.on('click', () => requestAction('show'));

let menu = new nw.Menu();
menu.append(new nw.MenuItem({ label: 'Exit', click: () => requestAction('exitApp') }));
tray.menu = menu;

Initialized via package.json:

"bg-script": "lib/app/trayIcon.js",

#7230

@rogerwang rogerwang added the nw2 label Nov 22, 2019
@rogerwang
Copy link
Member

Could you please provide a full sample (not code snippets) so I can reproduce? I tried a sample but the behaviour is the same between nw1 and nw2.

@Luzzifus
Copy link
Author

Luzzifus commented Dec 30, 2019

@rogerwang I tested again with 0.43.2.

The problem is actually not the bg-script being detached. As it seems to me, the document-end event is not fired anymore in nw2 mode when reloading the app via CTRL + SHIFT + R in the dev tools window.

Further explanation: My app spawns and manages a variable amount of child windows on startup (they're frameless, transparent and have no taskbar icon, so they can only be managed by the app). It keeps handles of all child windows and when the app is closed via either the main window close button or the tray icon exit option, it closes and destroys all child windows and the tray icon before closing itself. This still works fine.

Now for the case of reloading via dev tools: I have handled this case by using the "document-end" event of the main app window to do all the child windows / tray icon closing stuff before the main app is reloaded. This only works with nw2 disabled. With nw2 enabled, the "document-end" event is not being fired. This results in the child windows and tray icon from before reloading the app remaining open, so that I have all child windows doubled after reloading.

I tested this by changing the background color of the child windows instead of closing them in the "document-end" event handler. I only saw the changed background with disabled nw2 mode.

I can't seem to find any changes about the "document-end" event in the documentation, so I'm not sure if I need to adjust something or if this is indeed a bug.

Because of all the child window spawning, providing you with a working code example would be a bit of work. If you absolutely need to see it in action, I could give you access to the private repository of my app. Just let me know.

@rogerwang
Copy link
Member

Could you write a small case to verify your thoughts? I just tried a case which depends on 'document-end' and it works well: https://github.com/nwjs/nw.js/tree/nw43/test/sanity/document-start-end

@Luzzifus
Copy link
Author

I investigated a bit further and found out:

"document-end" will not be fired when I register it in jQuerys .ready() function. I attached a small test app, which includes two .html files of which only one is loaded via init.js. For one html-file "document-end" works as expected, for the other one it doesn't. Switch between both files in init.js and then reload the app in the dev tools with CTRL + SHIFT + R.

Again, it works for both test cases if nw2 is disabled.

docEnd_test.zip

@rogerwang
Copy link
Member

First, the sample code doesn't have the event fired before reload -- it's not supposed to fire because when the event handler is registerred in jQuery's ready(), it's already late.

With NW1 the event is fired after reload. But it is from the handler in the previous Window object, which should be destroyed when the reloading happens. It's fixed in NW2.

btw, in NW1 when you reload it again the event handler is called twice.

@rogerwang
Copy link
Member

closing for now since there is no actual bug here.

@Luzzifus
Copy link
Author

Luzzifus commented Jan 1, 2020

@rogerwang

First, the sample code doesn't have the event fired before reload -- it's not supposed to fire because when the event handler is registerred in jQuery's ready(), it's already late.

Late for what? The actual reload happens way after the execution of the .ready() function. Of course the registered event does not fire for the reload which CAUSED the execution of the .ready() function. That's not what it's supposed to do. It is supposed to register an event handler for a reload which is initiated AFTER that.

With NW1 the event is fired after reload.

It is not though, since if it were like that, my child windows from the previous instance would not be destroyed correctly. Also, if the event handler is registered correctly, shouldn't it fire the event independent of when it was registered? I mean even if I register the handler dynamically when the user clicks a button (just an example) shouldn't it fire when the app actually reloads at some point of time after that? I really don't understand why it doesn't fire when I register it later. Also that would severely limit the usefulness of that event.

How am I supposed to use it then to do what I want to do, that is destroying child windows before reloading? Keep in mind, this DOES work for my app when disabling nw2.

btw, in NW1 when you reload it again the event handler is called twice.

I don't see two alerts no matter how often I reload.

@rogerwang
Copy link
Member

By 'late' I mean the handler is registered after the point when the event should be fired.

When a page reloads, all the JS objects in it should be destroyed, including the event handler. A correct way to survive the page reload is to put the code in the background page, where 'init.js' is running.

@Luzzifus
Copy link
Author

Luzzifus commented Jan 1, 2020

Thanks for your patience!

A correct way to survive the page reload is to put the code in the background page, where 'init.js' is running.

This does actually fire the events all the time. One step forward. But when I try to close my child windows in the background page, I get an exception "extension context invalidated".

When using "document-end", my model was already reset and the child window objects/handles were gone. So I used "document-start". Now the handles are still available but the windows still can't be closed because of the exception.

init.js:

let closeActiveWidget = widgetModel => {
  if (typeof widgetModel.win == 'object')
    widgetModel.win.close();  // <-- this causes the "extension context invalidated" exception
}

let cb = win => {
  win.on('document-start', () => {
    model.activeWidgets.m_children.forEach(closeActiveWidget);
    win.removeAllListeners();
  });
}

nw.Window.open('index.html', {width: 850, height: 600}, cb);

@rogerwang
Copy link
Member

This could be another issue. You can file a new one with reproducible sample. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants