Allan, all of the settings are document-wise. There is a default for new documents, and saving the settings of the current document to be the default is what the save current setting as default does. So, I don't really think that any of this belong to the App Menu. All of this is because maintainers have tried hard to not to have a preferences dialog in Evince.

OK, that's cool. Toolbar is application-wide though, right?

Help, About and Quit would be good for consistency with other apps.

We've always rejected 'quit' so far (bug 305305), and I don't think we should add it now either.

I think the menu support in GTK only works for GtkApplicationWindow. We don't use either GtkApplication nor GtkApplicationWindow.

Yes. However in gtk master it's easy to just replace

gtk_main()

with

app = gtk_application_new (NULL, GTK_APPLICATION_NON_UNIQUE);
gtk_application_run()

and make EvWindow inherit from GtkApplicationWindow instead of GtkWindow.

There's a problem with gmenu not supporting context menus (yet?) so we'd have to keep using 2 different menu technologies for the app menu / menubar and the context menu... ugly :-(

(In reply to comment #5)
> There's a problem with gmenu not supporting context menus (yet?) so we'd have
> to keep using 2 different menu technologies for the app menu / menubar and the
> context menu... ugly :-(

It is possible to use gtk_menu_new_from_model() for popup menus.

This is working on the "wip/app" branch; I just added a rudimentary app menu, since it wasn't clear exactly which items should be in it and which not.

I should note that the branch contains quite a bit more than just the straigt g[tk]application port and adding an app menu; I've gone a bit overboard maybe on general cleanup first and then also porting to GResource :-) It would be a bit hard to separate the follow-up cleanup from the gapplication/app menu stuff, but they're all in self-contained commits of course to ease review, testing and bisectability.

Also I ported to gdbus-codegen first because IMHO it makes the code simpler and more easy to follow, even if the extra objects required look a bit odd. And it fits better into the GApplication DBus hooks setup to do it with extra objects.

The patch set is lightly tested (ie running with it for a bit, trying to open multiple new docs etc), and all bugs encountered by that were fixed. There could of course still be others lurking ;-)

I'd like also to add that I, too, am not very convinced about app menus in general, so this patch set makes *no effort at all* to remove *any* menu items or the whole menus from evince (and I *don't* think we should do that in the future, either!); and it only exposes a minimal set (Print, About, Help, Close window) from the app menu for now.

I've merged into master all commits except the GtkApplication and app menu bar ones. I'm still not sure we want it. I think it's very weird having some options in the window menubar and some others in the shell panel, maybe it makes sense for other applications, but not for evince, IMO. Maybe Help and About can be in the shell panel, but all other options belong to the instance window. It also breaks all users of focus follows mouse option of the window manager.

(In reply to comment #9)
> I've merged into master all commits except the GtkApplication and app menu bar
> ones. I'm still not sure we want it. I think it's very weird having some
> options in the window menubar and some others in the shell panel, maybe it
> makes sense for other applications, but not for evince, IMO. Maybe Help and
> About can be in the shell panel, but all other options belong to the instance
> window. It also breaks all users of focus follows mouse option of the window
> manager.

I'd say that Help, About and Quit are the bare minimum for the app menu. There's a push this cycle to make sure that applications have this set of entries in their app menus; it's important for applications to be consistent in this regard.

I agree on being consistent with other apps as long as it doesn't break valid use cases like focus follows mouse window manager setting, for example. Evince doesn't have a Quit action, and I'm not sure we want it either. I woulnd't mind if Help and About are in the shell panel since they are common to all window instances, but I think this should only happen when the window is maximized, otherwise those options are impossible to reach for focus follows mouse users.

(In reply to comment #11)
> otherwise those options are impossible to reach for focus follows mouse users.

This is a Shell bug that will be fixed, not a reason for applications to add inconsistent hacks to work around the issue.

we haven't added any hack, we simply don't provide an app menu for now.

(In reply to comment #11)
> I agree on being consistent with other apps ...

So be consistent. What you're proposing would make Evince different to *every* other app. That's inevitably going to cause confusion. People will get used to these entries being in the app menu. They will expect to find them there.

> ... but I think this should only happen when the window is maximized,
> otherwise those options are impossible to reach for focus follows mouse users.

That is such a bad idea... it breaks pretty much every usability rule in the book. It's not just inconsistent between apps, but also inconsistent within Evince. You'd be breaking the experience for everyone, since you'd be making them uncertain about what should be universal patterns.

As a matter of fact, we've been working on a solution for focus follows mouse - it's a bug for the whole of GNOME, so please don't try and fix it in *one* app.

(In reply to comment #13)
> we haven't added any hack, we simply don't provide an app menu for now.

Right, but I was referring to the consideration of disabling the app menu for non-maximized windows.

(In reply to comment #15)
> (In reply to comment #13)
> > we haven't added any hack, we simply don't provide an app menu for now.
> 
> Right, but I was referring to the consideration of disabling the app menu for
> non-maximized windows.

ah, ok, I think adding a small delay in app menu change is indeed a good idea, so I'm looking forward to seeing that patch merged into gnome-shell.

(I need to rebase the branch after all that cherry-picking; I'll do that when I have a bit of time, soon.)

Branch has been rebased.

I've merged most of the branch, but not the app menu commits, because I'm still not convinced it's a good idea for Evince. The only action I would add to the app menu is help, and I'm not sure it's worth the effort just for the help. About dialog is attached to a particular window, it's very weird using a global action that shows a dialog attached to a "random" window. Which one is the "active" window?

I wouldn't mind if the app menu thing is implemented, if only Help is in the app menu. But instead of using a (so complex) ui manager adapter, help could be a global action implemented in EvApplication (ev_application_show_help() or something like that). then both GtkActions in EvWindow and GAction in the app menu would simply call ev_application_show_help().

Rebased and reworked the branch.

Please implement a fallback menu that makes sense for those who don't use GNOME Shell. The default fallback app menu is an usability disaster. It shouldn't be on ANY application.

Maybe you should look at what the branch actually does? It only *adds* an app menu; no regular menus were removed. And the app menu is only installed when the shell is running; no ugly fallbacks.

Sorry, I can't understand very well the code. But if the ugly fallback doesn't appear, then it's fine. I was asking because some apps already have the ugly fallback.

Pushed to master.

We decided to keep the previewer appmenu-less; the existing patch kept on the wip/app branch for reference.

*** Bug 693962 has been marked as a duplicate of this bug. ***

>We've always rejected 'quit' so far (bug 305305), and I don't think we should
add it now either.

That bug is from 2005!!!. Even after 8 long years (Gnome changes, technology changes, usability changes, UI changes) and after so many duplicate bugs you don't feel the need to rethink that position?

>About dialog is attached to a particular window

The about dialogue shows the evince version, poppler backend version, licensing and credits. Which of that is window specific? 

May be you are confusing with the properties window?

(In reply to comment #27)
> >About dialog is attached to a particular window
> 
> The about dialogue shows the evince version, poppler backend version, licensing
> and credits. Which of that is window specific? 

Yes, the concept is global and makes sense in the global app menu, but it's implemented as a modal dialog attached to a particular evince window.

> May be you are confusing with the properties window?

No

(In reply to comment #28)
> .....but it's implemented as a modal dialog attached to a particular evince window.

Most of the gnome applications (nautilus, epiphany, gnome-documents,...) has about dialogue attached to specific windows. And all of them has about dialogue in the app menu.

*** Bug 701485 has been marked as a duplicate of this bug. ***