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

Use the same internal implementation of the browser native window #5875

Open
rogerwang opened this issue Apr 26, 2017 · 43 comments

Comments

Projects
None yet
7 participants
@rogerwang
Copy link
Member

commented Apr 26, 2017

Currently the native window of NW application is implemented with the one from Chrome App. It need to be changed to the implementation of the Browser native window.

@Yonezpt

This comment has been minimized.

Copy link

commented Feb 26, 2018

Quick question regarding this subject, will this enable tabs to be opened/used or it will not change in this regard?

The advantage of this would be for them to have their own process instead of being iframes tied to the same window process. (#242)

EDIT: Only now I noticed this issue was already one year old, apologies.

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2018

Yes. It could help on enabling tabs as in Chrome browser.

@elpeleq42

This comment has been minimized.

Copy link

commented Dec 31, 2018

@rogerwang How do you plan on implementing tabs? Will we be able to program then just like we do with menubar?

Also, will each tab have it's own instance of the menubar?

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Jan 2, 2019

The plan is to reuse the tab provided by Chrome browser and the API as well: https://developer.chrome.com/extensions/tabs . Please continue the discussion about tabs and make your proposal in #1136. Thanks.

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Jan 4, 2019

0.35.4 is just released with a testing version of this. Please help testing it: https://nwjs.io/blog/v0.35.4/ . Currently it's under --enable-features=nw2 switch. The plan is to make the switch default after 2-3 major releases and then start to remove the old implementation later.

@TheRealDannyyy

This comment has been minimized.

Copy link

commented Jan 4, 2019

Would very much appreciate a new page in the manual, regarding all the changes and new features.

@elpeleq42

This comment has been minimized.

Copy link

commented Jan 4, 2019

My windows 10 blocked the startup of my application when I changed it to the last version and used the flag(was working with v0.35.2)
And even after unblocked, it took a long time to show up the screen(A loading icon showed before my icon, I think that could be removed. It also shows when I refresh the application). Other windows opened by the main window also take a lot longer to load.

It seems that CTRL+S, CTRL+O and other shortcuts are always opening the file dialogs. And new undesired options were added to the right-click both inside the window and on the top border(Things such as reload, open in browser, print, zoom, task manager,etc) .

Also, the window seems to be a bit bigger than before, but that is not a problem.

Edit: Oh would you look at that, CTRL+F opens the find tool of chrome. Not interesting for my applicatron but cool.

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Jan 4, 2019

@elpeleq42

This comment has been minimized.

Copy link

commented Jan 4, 2019

Could you please provide a reproduce?

On Fri, Jan 4, 2019, 10:45 AM elpeleq42 @.*** wrote: My windows 10 blocked the startup of my application when I changed it to the last version(was working with v0.35.2) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5875 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKGGfEik-vmUGTPEIV9uAavRSamKnCaks5u_sBLgaJpZM4NIZYq .

Well by simply downloading and executing it, without any application loaded and not even adding the flag, Windows 10 tries to block nwjs. Then by adding the flag to package.json and loading any html file, all the problems I said with exception of the slow loading shows up.

Loading my application(which is "heavy", having nearly 2MB of javascript to be loaded and parsed) now doesn't seem to have any big extra delay anymore(there's just a small one, I wold say like 100ms extra time to load). I think it was the first startup of NW.js with that flag that had a really slow loading, but once chromium cache up stuff, it doesn't have anymore. Or maybe it was a problem with my computer, but I couldn't reproduce it anymore.

EDIT: Forgot to say: all of that tested in the "normal" build

@Blatman

This comment has been minimized.

Copy link

commented Jan 5, 2019

OSX 10.13.6 (High Sierra)
Transparency broken - tried the new switch with 0.35.0-sdk and 0.35.5-sdk.
0.35.0 Works OK with --enable-features=nw2 --disable-gpu-compositing --force-cpu-draw
0.35.5 fails with --enable-features=nw2 --disable-gpu-compositing --force-cpu-draw
will only show window bar.
If I remove compositing switch the window body will show but is broken
Each test done after app profile deleted.
Sample app attached with snaps -

TransTest-simple.zip

Fail - 0.35.5
fail-0 35 5-nw2

OK - 0.35.0
ok-0 35 0-nw2

Additional Notes:
App used to open in center of screen but now opens in top left of screen.

Window sizing appears more accurate (on OSX at least). Without the switch the window height appeared to exclude the top window bar ( so height + 22px) but with the switch the height is the actual window height specified (top window bar inclusive).

setTitleInternal error -

error

this proved to be this bit of code -
96 win.title = nw.App.manifest.name;
if line 96 was remmed out app worked OK.

Link status messages appear at bottom of window (eg: when tabs are hovered/clicked) - this is OK for a browser but not really desirable in desktop app -

linkstatusbar

@elpeleq42

This comment has been minimized.

Copy link

commented Jan 5, 2019

I can confirm status messages appearing on Windows 10 too.

rogerwang added a commit to nwjs/chromium.src that referenced this issue Jan 6, 2019

rogerwang added a commit that referenced this issue Jan 6, 2019

rogerwang added a commit to nwjs/chromium.src that referenced this issue Jan 6, 2019

@Blatman

This comment has been minimized.

Copy link

commented Jan 6, 2019

Corker - just tried above fixes with latest and all fixed OK (I tested OSX only).

Just noted - Quit App(CMD-Q ) from top OSX menu is not working - right-click quit from dock icon works OK.
This is reproducible with above test app and appears to have happened from the original new switch build.

@Blatman

This comment has been minimized.

Copy link

commented Jan 7, 2019

Just noticed in another app that doing stuff when closing windows appears broken when the switch is implemented. The app windows will close but the interim action (clearing a folder not done).

Relevant code -

var mainWin = nw.Window.open('index.html', {
"fullscreen": false,
"resizable": false,
"title": "",
"frame": true,
"width": 1295,
"height": 1322,
"position": "center",
"show": true
//});
}, function (mainWin) {

mainWin.on('loaded', function () {
    console.log('Background Window - Launched!!!!');
    mainWin.focus();
    mainWin.setAlwaysOnTop(true);
});

mainWin.on('closed', function () {
});

mainWin.on('close', function () {
    console.log('Closing ......')
    clearFolder();  // app & all win will close when cleared
});

});

async function clearFolder() {
var target = path.join(localStorage.pref_appUserDataPath, 'wkgFolder');
try {
await fse.emptyDir(target)
console.log('wkgFolder Cleared!')
} catch (err) {
console.error(err)
}
nw.App.closeAllWindows();
nw.App.quit();
mainWin = null;
}

This code is from startApp.js (as set in main of package.json) and HTML window is opened by this - all OK if the switch is not used - tested with 0.35.5-sdk. Basically it is clearing a transient folder when app closed. Same occurs with 0.36.0b1 which appears to implement nw2 also.

Also window position: center is ignored and opens top left of screen.

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Jan 7, 2019

@Blatman

This comment has been minimized.

Copy link

commented Jan 7, 2019

@rogerwang - The app closes OK but the clearFolder() doesn't happen. In the code above the actions under mainWin.on('loaded', etc work OK but nothing appears to happen with the 'closed' or 'close' options but they do work alright without the switch. I set an alert inside the 'close', function and it was not triggered at all with the switch in use but triggered when switch not used.

rogerwang added a commit that referenced this issue Jan 8, 2019

rogerwang added a commit that referenced this issue Jan 8, 2019

@Blatman

This comment has been minimized.

Copy link

commented Jan 9, 2019

@rogerwang - thanks for close event fix it now allows 'tidyup' code to work however I find it now takes 2 clicks to close the window and CMD-Q on menu still not working.
I have attached a wee test app to demonstrate.
The simple mode should just invoke nw.App.closeAllWindows() on the close event but isn't honoured (neither is nw.App.quit()).
With the html and js start modes using the 'tidyup' code the first window close will do the tidy up (clear wkgFolder) but window remains open until second click.
Test app currently set for simple test.

nw2test.zip

Window position: center still not honouring.

Also noted that right-click context menu will offer saveas webpage option as per browser function - this didn't happen prior to switch.

contextmenu-normal

Cheers

rogerwang added a commit that referenced this issue Jan 10, 2019

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Jan 12, 2019

@tolrtd could you please provide a reproduce? It works for me.

@tolrtd

This comment has been minimized.

Copy link

commented Jan 13, 2019

@rogerwang I think it is when you have "show": false in the window manifest. In the attached example the Main Window should open fine, but the Child Window does not (on my Win 10 x64, SDK version, it freezes and does either white screen or sometimes black screen, no frame)

nwjs-5875-test.zip

rogerwang added a commit to nwjs/chromium.src that referenced this issue Jan 14, 2019

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

@tolrtd I just fixed it in git and it will be available in the next nightly build.

@Blatman

This comment has been minimized.

Copy link

commented Jan 18, 2019

Just discovered that using a nativemenu on Mac OS will break CMD-Q/Quit from menubar
Updated test app attached - it starts on index-simple.html and uses osxmenu.js

Essential code -
var nw,
win = nw.Window.get();

var nativeMenuBar = new nw.Menu({
type: "menubar"
});

nativeMenuBar.createMacBuiltin('appname', {
});

win.menu = nativeMenuBar; // CMD-Q works ok if this line remmed out

nw2testmenu.zip

OSX 10.13.6
Tested with 0.35.6-sdk and 0.36.0-sdk

rogerwang added a commit to nwjs/chromium.src that referenced this issue Jan 18, 2019

nw2: macOS: fix Cmd-Q with user menu bar
port closeAllWindowsQuit; Ref nwjs/nw.js#5875
@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Jan 18, 2019

@Blatman Thanks. Fixed only on nw36 branch since it's the new nightly and will be the next release.

rogerwang added a commit to nwjs/chromium.src that referenced this issue Jan 19, 2019

nw2: macOS: fix Cmd-Q with user menu bar
port closeAllWindowsQuit; Ref nwjs/nw.js#5875
@TheRealDannyyy

This comment has been minimized.

Copy link

commented Feb 3, 2019

I've tested the new implementation and found some issues with it, in comparison with the existing one.
(Tested using NWjs v0.36.0 and this Sample Project.)

Fullscreen Mode:

  • F11 fullscreen key should be blocked by default
  • Fullscreen notification should not be displayed
  • Moving mouse pointer to top of the screen should not display "X" button
@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Feb 4, 2019

@TheRealDannyyy thanks. Will fix those issues.

@TheRealDannyyy

This comment has been minimized.

Copy link

commented Feb 27, 2019

Referencing #5337 for potential future fix.

@Blatman

This comment has been minimized.

Copy link

commented Mar 16, 2019

@rogerwang Would it be worth pinning this issue until closed since it is P1 and a new feature? - it tends to get lost in the issues list and folk are creating separate issues that relate to the nw2 switch.

@rogerwang rogerwang pinned this issue Mar 17, 2019

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2019

@Blatman done

@TheRealDannyyy

This comment has been minimized.

Copy link

commented Mar 28, 2019

@rogerwang I've tested the most recent nw.js v0.37.1 release and there is still one small fullscreen issue left. Moving the mouse pointer to top of the screen still displays the "X" button.

I believe this feature has a flag so it might just require that flag to be disabled by default.

fullscreenclose

rogerwang added a commit to nwjs/chromium.src that referenced this issue Apr 10, 2019

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

@TheRealDannyyy I just fixed the fullscreen button issue in git. It will be available in the next nightly build.

@TheRealDannyyy

This comment has been minimized.

Copy link

commented Apr 15, 2019

@rogerwang Thanks for the fix! Can confirm that all fullscreen issues have been fixed.

The only major issue left is the window dimension issue as previously mentioned. If you could fix or provide a workaround for that would be amazing. (Tested using NWjs v0.37.4 and this Resolution_Test.zip.)

Reproduction:

  1. Run with most recent version of NW.js
  2. Notice wrong window dimensions being used
    (Left and right sides of the canvas.)

Screenshot:
Issue

@rogerwang

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2019

@TheRealDannyyy I don't notice any difference under the 2 modes with 0.38.0. Could you try again?

@TheRealDannyyy

This comment has been minimized.

Copy link

commented Apr 30, 2019

@rogerwang I can still reproduce using nw.js v0.38.1, nothing has changed for me.

@Blatman

This comment has been minimized.

Copy link

commented Apr 30, 2019

@rogerwang - FWIW I tried the test zip with 0.38.2-sdk and get the same as screenshot above but only if nw2 enabled in manifest - just get a black cross without nw2 which I guess is the desired outcome.
10.13.6 used.

@Blatman

This comment has been minimized.

Copy link

commented May 6, 2019

Just noticed - maybe a patch still to go into alpha build?
High Sierra 10.13.6

Using 0.39.0a1 I get Uncaught TypeError: win.getPrinters is not a function - was OK on 0.38.2 and earlier. Works OK without nw2 feature enabled. Normally should get a printer json object if win.getPrinters(console.log) done.

winGetPrinters Bug.zip

rogerwang added a commit that referenced this issue May 14, 2019

@Blatman

This comment has been minimized.

Copy link

commented May 15, 2019

Bonza - getPrinters works fine

@Blatman

This comment has been minimized.

Copy link

commented May 21, 2019

Getting Uncaught Errors concerning onDocument properties with 0.39 (only with nw2 enabled) - not sure if this is going to be a problem ....
No errors if 0.38.4 is used.

error

Test app will show errors in background-context

nw2test.zip

rogerwang added a commit that referenced this issue Jun 2, 2019

@Blatman

This comment has been minimized.

Copy link

commented Jun 5, 2019

@rogerwang - not sure if this last commit was meant to address the Uncaught errors (BLESSED_EXTENSION) but still happens with latest build (Index of /live-build/nw39/06-05-2019/9836f0de6/). Note: the errors have occurred since the first 0.39a1 build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.