Probably makes sense, however thinking of translators I'm wondering if strings could be synchronized with existing ones like
  #: ../shell/main.c:74
  msgid "Run evince in fullscreen mode"
  #: ../shell/main.c:75
  msgid "Run evince in presentation mode"
(which does not necessarily imply to change the patch, but maybe to change ../shell/main.c instead)

(In reply to André Klapper from comment #1)
> Probably makes sense, however thinking of translators I'm wondering if
> strings could be synchronized with existing ones like
>   #: ../shell/main.c:74
>   msgid "Run evince in fullscreen mode"
>   #: ../shell/main.c:75
>   msgid "Run evince in presentation mode"
> (which does not necessarily imply to change the patch, but maybe to change
> ../shell/main.c instead)

Hmm, these are help strings for command line options, so they influence the startup behaviour. Not a strong opinion, just tell me if you want me to change the patch :-)

Review of attachment 357819 [details] [review]:

Thanks for the patch. I think it makes sense, but I have some comments. We are in string freeze so we have to wait to after the freeze.

::: shell/help-overlay.ui
@@ +63,3 @@
+              <object class="GtkShortcutsShortcut">
+                <property name="visible">True</property>
+                <property name="title" translatable="yes" context="shortcut window">Enter fullscreen</property>

I would put "Enter/exit fullscreen" or "Toggle fullscreen" or "Fullscreen on/off" 
FWIW, Gedit uses "Fullscreen on/off".

@@ +73,3 @@
+                <property name="accelerator">F5</property>
+              </object>
+            </child>

You are missing a shortcut for Esc to Exit presentation mode.

Wow, you should definitely change your review process, that's almost 6 months! As I don't have any time atm this will have to wait.

@Jonas: Developer are volunteers, hence same problem with reviews which sometimes "have to wait" unfortunately. :-/

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

(In reply to André Klapper from comment #5)
> @Jonas: Developer are volunteers, hence same problem with reviews which
> sometimes "have to wait" unfortunately. :-/

I know that, being a volunteer myself in other projects. I'm just saying that long review cycles will distract possible one-time contributors. And it's not like there is no activity in the evince git repository...

This is assigned to the user documentation. The patch is against the UI code, reassigning.

Created attachment 369323 [details] [review]
Add shortcuts for fullscreen and presentation mode

(In reply to José Aliste from comment #3)
> Review of attachment 357819 [details] [review] [review]:
> 
> ::: shell/help-overlay.ui
> @@ +63,3 @@
> +              <object class="GtkShortcutsShortcut">
> +                <property name="visible">True</property>
> +                <property name="title" translatable="yes" context="shortcut
> window">Enter fullscreen</property>
> 
> I would put "Enter/exit fullscreen" or "Toggle fullscreen" or "Fullscreen
> on/off" 
> FWIW, Gedit uses "Fullscreen on/off".

Done.

> @@ +73,3 @@
> +                <property name="accelerator">F5</property>
> +              </object>
> +            </child>
> 
> You are missing a shortcut for Esc to Exit presentation mode.

Not listed in Gedit either, so I didn't add it for consistency.

(In reply to Jonas Hahnfeld from comment #9)
> Created attachment 369323 [details] [review] [review]
> Add shortcuts for fullscreen and presentation mode
> 
> (In reply to José Aliste from comment #3)
> > Review of attachment 357819 [details] [review] [review] [review]:
> > 
> > ::: shell/help-overlay.ui
> > @@ +63,3 @@
> > +              <object class="GtkShortcutsShortcut">
> > +                <property name="visible">True</property>
> > +                <property name="title" translatable="yes" context="shortcut
> > window">Enter fullscreen</property>
> > 
> > I would put "Enter/exit fullscreen" or "Toggle fullscreen" or "Fullscreen
> > on/off" 
> > FWIW, Gedit uses "Fullscreen on/off".
> 
> Done.

We don't use "on/off" for anything that couldn't be a physical switch. Please use "Toggle fullscreen"

Created attachment 369326 [details] [review]
Add shortcuts for fullscreen and presentation mode

Another reviewer, another opinion

(In reply to Jonas Hahnfeld from comment #11)
> Created attachment 369326 [details] [review] [review]
> Add shortcuts for fullscreen and presentation mode
> 
> Another reviewer, another opinion

Thanks, Bastien, Do we need a review from someone at the doc team? Or can I just merge this?

(In reply to Jonas Hahnfeld from comment #11)
> Created attachment 369326 [details] [review] [review]
> Add shortcuts for fullscreen and presentation mode
> 
> Another reviewer, another opinion

I'm sorry gedit has it wrong there ;)

(In reply to José Aliste from comment #12)
> (In reply to Jonas Hahnfeld from comment #11)
> > Created attachment 369326 [details] [review] [review] [review]
> > Add shortcuts for fullscreen and presentation mode
> > 
> > Another reviewer, another opinion
> 
> Thanks, Bastien, Do we need a review from someone at the doc team? Or can I
> just merge this?

We're in hard code freeze, but even last week, that would have required a string freeze break, as this added new strings to translate.

Looks fine to commit after branching for the 3.28 release though.

Sure, I already branched for 3.28 in order to commit this only to master. THanks

Review of attachment 369326 [details] [review]:

I am fine with this.

This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.