1381231 - Firefox does not recognize xf86reload

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[Jonas L. B.]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170612122310

Steps to reproduce:

Press the reload key, bound to XF86Reload. In my case this is on a Chromebook running GalliumOS. 


Actual results:

Firefox does nothing whatsoever.


Expected results:

Firefox should reload the page (as when pressing F5).

An old bug actually mentions this but it was mistakenly closed (only a subset of it was implemented): https://bugzilla.mozilla.org/show_bug.cgi?id=376206
In particular, it sounds like firefox handles xf86refresh as it should, but not xf86reload. These should probably map to the same.

Comment 1

[Justin Dolske [:Dolske]]

Guessing this is actually a widget bug?

Comment 2

[Adrián L.]

Attachment: Adding XF86Reload

This patch fixes it for me.

Comment 3

[Adrián L.]

Comment on attachment 8937438 [details] [diff] [review]
Adding XF86Reload

diff -ura a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp
--- a/widget/gtk/nsWindow.cpp	2017-12-18 12:52:22.504435000 +0100
+++ b/widget/gtk/nsWindow.cpp	2017-12-19 10:53:58.018830655 +0100
@@ -3056,6 +3056,7 @@
         case GDK_Forward:
             return DispatchCommandEvent(nsGkAtoms::Forward);
         case GDK_Refresh:
+        case GDK_Reload:
             return DispatchCommandEvent(nsGkAtoms::Reload);
         case GDK_Stop:
             return DispatchCommandEvent(nsGkAtoms::Stop);

Comment 4

[Adrián L.]

Attachment: Add XF86Reload 2

Sorry, the previous comment 2 was an error. Meant to attach this improvement of the patch.

Comment 5

[Glen Whitney]

The issue appears still to exist in Firefox 81.0b4. Given that patches were provided earlier to rectify this issue, what might I do to help get a fix into the stream for release? Would submitting code via Phabricator (if I have that spelled right) help? Thanks for letting me know, happy to put a bit of effort into this help get a fix in. Thanks!

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Yeah, the patch looks fine to me, it's just a matter of submitting it for review (though I'm not a gtk peer). If you send the patch to phabricator with r?stransky, Martin can review and check it in :)

Comment 7

[Glen Whitney]

Attachment: Bug 1381231 - Add GDK_Reload to the keyvals triggering Reload r=stransky

Comment 8

[Glen Whitney]

OK, I tried my hand at it. The process seems a bit complex (which is understandable, as Firefox is a much larger project than other open-source efforts I've contributed to) so I hope I've done things right so far. I am slightly worried by the annotation just above that the Assignee has been changed to me; is there anything else I need to do to move this along? Thank you so much for your advice so far!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0de10c26da55
Add GDK_Reload to the keyvals triggering Reload r=stransky

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/0de10c26da55