Mac PWAs: File > New opens Chrome window inappropriately |
|||||
Issue descriptionIn a PWA, the File menu has a "New Window" item. This menu item opens a new Chrome browser window. We should decide if this should menu item should (1) be removed or (2) open a new instance of the PWA window (as it would if we were to launch again from chrome://apps).
,
Dec 17
+ markchang@ since this seems like a product decision
,
Dec 20
Both - New Window - Open File don't seem to provide relevant functions for PWAs, and the outcomes (open a browser window, open a file in the window frame with no connection to the app). pcovell@, mgiuca@, markchang@ - can we make a call to not have these two menu items?
,
Dec 20
Yeah I think we get rid of "Open file" altogether (since a PWA might be a file editor where there'd be an expectation of "Open file" opening a file *in* the app, not opening a web page in the browser). [Aside: I had no idea we had this feature. I don't see it on the Chrome menu on other platforms. Is this a macOS-only feature?] "New Window" should open a new PWA app window at the start page. If that's too hard, we should nix it for this version, but bring it back in future.
,
Dec 21
We hook that up to IDC_NEW_WINDOW at: https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/main_menu_builder.mm?rcl=491e2671e35208ba3654d4211c775708df5c52f2&l=96 We then handle IDC_NEW_WINDOW by calling NewWindow at https://cs.chromium.org/chromium/src/chrome/browser/ui/browser_command_controller.cc?rcl=a4bd5ce4b8ed521ad948786963eacf35360405f4&l=344 And NewWindow calls NewEmptyWindow at https://cs.chromium.org/chromium/src/chrome/browser/ui/browser_commands.cc?rcl=9d37d5147d8b9d9a20bd1b55f2f7daad06ed0c3e&l=579 If we see that the browser is a hosted app, we should not create a NewEmptyWindow there. Doing the same thing we do for regular app launch (whatever that is) should work. When we launch from a shim we do it at https://cs.chromium.org/chromium/src/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc?rcl=25578467440d7e7261f1a1a5c32173e561b57d8a&l=235 I'd test that, but I installed Mojave and, in exchange for awesomely-rounded PWA windows, my build is hosed.
,
Dec 21
Yeah, it should read CreateApplicationWindow there (possible with some ifdefs sprinkled in).
,
Dec 21
,
Dec 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1860807b58e924ffc973ae03f3b2556361dddf2c commit 1860807b58e924ffc973ae03f3b2556361dddf2c Author: Christopher Cameron <ccameron@chromium.org> Date: Fri Dec 21 15:34:28 2018 MacPWAs: Fix File>New and File>Open Remove the Open item from the File menu -- the item isn't particularly meaningful. Make New Window in the File menu open a new instance of the PWA. Of note is that the window opens in the exact same place as the initial window of the PWA (as opposed to being offset), which should be fixed. Bug: 913360 Change-Id: I606852e4d8092046b6194870df22400b2eec0b1e Reviewed-on: https://chromium-review.googlesource.com/c/1388003 Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#618529} [modify] https://crrev.com/1860807b58e924ffc973ae03f3b2556361dddf2c/chrome/browser/ui/browser_commands.cc [modify] https://crrev.com/1860807b58e924ffc973ae03f3b2556361dddf2c/chrome/browser/ui/cocoa/main_menu_builder.mm
,
Dec 21
I've filed issue 917416 on the fact that new windows show up in non-ideal (not tiled) places, as a non-P1. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dominickn@chromium.org
, Dec 12