I have the same issue, but I did not have it in Ubuntu Gnome 15.10, where I used Gnome 3 Staging PPA to update to Gnome 3.18. But I don't know which version of Web was included there.

But now it's like this:

I used Web to create a web application (love this feature! thank you!) in a brand new fresh install of Ubuntu Gnome 16.04 updated to Gnome 3.20 with the Gnome 3 Staging PPA. Web is at version 3.18.5

The strange thing that happens is that if I start writing, either a post or a PM, while holding Shift the first letter is input twice. I normally use Shift to capitalize the first letter in every sentence I write, so this is quite annoying.

I did try to turn on Caps Lock just to check, and it's the same thing: Now I get two non-capitalized letters in the start when holding shift. Using Caps Lock I can start the sentence with a capitalized letter by turning it on, typing the letter, and turning it off again. So it's not the capitalization that makes this happen, but the fact that I hold the Shift button.

It's only the very first letter typed into a text area that's affected in this way. I've also tested this in a web application of Google.com and it does not happen there.

== What I do ==

Write "Hello world", by holding Shift on the first letter.

== What I expect ==

The text "Hello world" to appear in the text area.

== What happens ==

The text "HHello world" appears

(In reply to Børge / forteller from comment #1)
> It's only the very first letter typed into a text area that's affected in
> this way.

Do you know of anywhere it can be reproduced that's not behind a login (i.e. not Facebook)? Text entries are definitely working just fine here on Bugzilla.

If this is happening on many sites, I will try to bisect it in Epiphany. If it's only reproducible on Facebook, then it doesn't seem like a priority issue to me, so someone who uses Facebook can try bisecting.

So I bisected and this change introduced the problem:

https://git.gnome.org/browse/epiphany/commit/?id=ebbc2351b76fd1644685a800ef0ffb635f92deb2

commit ebbc2351b76fd1644685a800ef0ffb635f92deb2

    window: properly propagate keys to the page
    
    Sometimes Epiphany wrongly steals the keyboard events
    when they were supposed to be handled by the web view.
    This is very clear on text editor pages such as Google
    Drive's documents, where we can't set e.g. bold text
    using Ctrl + B shortcut.
    
    Fix that by trying to propagate events with the form of
    (Ctrl|Alt|Shift)+letter to the focused widget.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=113449

I considered reverting the commit that caused this regression, but the code looks right to me. This requires further investigation.

I'm the obvious one to blame here. Since I don't have a Facebook account, and I'm not willing to create one, it'd be very nice to know if this happens outside Fb.com too so I can test and fix it.

I looked a bit into this and what is happening is that there are two keyboard events coming in. My guess is that Facebook is doing this with its JavaScript, but that would require further investigation.

In any case, I would say that it is arguably correct to treat Ctrl+letter in the same way as Shift+letter. Epiphany is never going to steal a Shift+letter event from the web application. I would remove that case from the bypass.

It could also be webkit, we re-inject keyboard events not handled by the web process to allow applications to handle them. In this case I think the web process should have handled it, though, since it has inserted a character. I would need to look at this in more detail.

This issue also exists on messenger.com, which is Facebook Messenger.

Where would one begin to look to fix this issue?

Comment #7 seems like the best lead to me. You'd have to dive into WebKit2's handling of GdkEvents.

OK, this seems to be fixed in Epiphany 3.22, independent of WebKit version. I don't know how or why. Can anyone confirm?

(In reply to Michael Catanzaro from comment #10)
> OK, this seems to be fixed in Epiphany 3.22, independent of WebKit version.
> I don't know how or why. Can anyone confirm?

OK, for difficult-to-explain reasons, it turns out this information was wrong. But it still seems to be fixed for me with Epiphany master and WebKit trunk. So I'll leave this in the NEEDINFO state, to see if anyone can reproduce it with Epiphany 3.23.1 and WebKitGTK+ 2.15.1 once they are released.

Seems to be fixed in WebKitGTK+ 2.14.0 or 2.14.1.

(In reply to Michael Catanzaro from comment #12)
> Seems to be fixed in WebKitGTK+ 2.14.0 or 2.14.1.

Still happening in Epiphany 3.22.1 and WebKitGTK 2.15.1

Alas.

I was able to reproduce this reliably on Riot two weeks ago, but can't anymore, not with Ephy 3.20 nor with Ephy master, not with WebKit 2.14.1 nor with WebKit trunk. :(

I cannot reproduce reliably (it sometimes doesn't happen) with Epiphany 3.22 on flatpak on facebook.com, but I can reproduce it on messenger.com.

(In reply to Michael Catanzaro from comment #14)
> Alas.
> 
> I was able to reproduce this reliably on Riot two weeks ago, but can't
> anymore, not with Ephy 3.20 nor with Ephy master, not with WebKit 2.14.1 nor
> with WebKit trunk. :(

Still happening 100% of the time for me in Riot, using Epiphany 3.22.2
(WebKitGTK+ 2.14.2) in Riot. There's clearly something wrong in the way
Epiphany is handling keyboard input, because the issue is no there in
Revolt (https://github.com/aperezdc/revolt), a small wrapper for Riot
which embeds a WebKitWebView _without_ doing any keyboard handling of
its own at all.

See comment #7:

(In reply to Carlos Garcia Campos from comment #7)
> It could also be webkit, we re-inject keyboard events not handled by the web
> process to allow applications to handle them. In this case I think the web
> process should have handled it, though, since it has inserted a character. I
> would need to look at this in more detail.

I don't think we have enough information yet, but Epiphany's code looks good and WebKit's code is quite complicated, so Epiphany is not my prime suspect.

(In reply to Adrian Perez from comment #16)
> Still happening 100% of the time for me in Riot, using Epiphany 3.22.2
> (WebKitGTK+ 2.14.2) in Riot.

I don't understand why upgrading to 2.14 fixes the issue for me. I really can only reproduce in 2.12. :S

(In reply to Michael Catanzaro from comment #17)
> See comment #7:
> 
> (In reply to Carlos Garcia Campos from comment #7)
> > It could also be webkit, we re-inject keyboard events not handled by the web
> > process to allow applications to handle them. In this case I think the web
> > process should have handled it, though, since it has inserted a character. I
> > would need to look at this in more detail.
> 
> I don't think we have enough information yet, but Epiphany's code looks good
> and WebKit's code is quite complicated, so Epiphany is not my prime suspect.

I can't follow. Could you explain your trail of thought here? I am telling
that using the same exact WebKitGTK+ version with both Epiphany and other
application, this bug exists in Epiphany only. Therefore, the issue _must_
be in Epiphany.

Try this:

  % cat > test.py <<EOF
  from gi.repository import WebKit2, Gtk
  import sys
  window = Gtk.Window()
  webview = WebKit2.WebView()
  window.add(webview)
  window.show_all()
  webview.load_uri(sys.argv[1])
  Gtk.main()
  EOF
  % python test.py https://facebook.com  # Or any other page with the issue

and you will see that keyboard input works *perfectly fine* in the window
opened by the script.

(In reply to Adrian Perez from comment #18)
> application, this bug exists in Epiphany only. Therefore, the issue _must_
> be in Epiphany.

Or, at least, be triggered by Epiphany's code.

Is it possible to be a simplest application that includes Ephy KB management code?

(In reply to Andres Gomez from comment #19)
> (In reply to Adrian Perez from comment #18)
> > application, this bug exists in Epiphany only. Therefore, the issue _must_
> > be in Epiphany.
> 
> Or, at least, be triggered by Epiphany's code.

Yes, what Andres says is what I meant. :) Maybe there is an error in ephy_window_key_press_event(), but if so I don't see it.

> Is it possible to be a simplest application that includes Ephy KB management
> code?

The Epiphany code is actually pretty simple, ephy_window_key_press_event() is a fairly short function. It shouldn't be hard to translate it to PyGObject. The tricky part is that it manually passes the event down to the webview, then, if the web view doesn't handle the event, chains up. I guess that could result in it getting sent to the web view a second time, but it would be a WebKit bug if the web view did something with the event and then propagated it on to Epiphany. This is totally speculative, I haven't investigated really, that's just what seems most likely to me. We just need to take the time to dive in.

At any rate, Georges's patch is correct, fixes a worse bug, and should not be reverted.

To be clear: the simple application works because it does not have any keyboard shortcuts of its own. Epiphany does not have that luxury. :)

(In reply to Michael Catanzaro from comment #21)
> To be clear: the simple application works because it does not have any
> keyboard shortcuts of its own. Epiphany does not have that luxury. :)

Soon I'll be adding some keyboard shortcuts to the Revolt window, so if
this manifests then I'll comment here. Or maybe I'll try to translate
“ephy_window_key_press_event()” to Python for testing before that.

Created attachment 339967 [details]
Test program

In the end it was easier to isolate ephy_window_key_press_event() in
a smaller C program than to port it over to Python. If you compile and
run the attached “test.c” program, the issue is still there. Check with:

  % gcc -std=gnu99 test.c `pkg-config webkit2gtk-4.0 --cflags --libs`
  % ./a.out https://facebook.com

Created attachment 339976 [details] [review]
window: bypass event propagation on composed keys

When pressing a composed key sequence (Ctrl|Shift|Alt + Key), the
current code allows propagating the event back to the focused widget.
When the webview itself is focused and we hit a specific case of
the event machinery, the event is propagated to the webview twice
as it bubbles up from the webview, hits the window, and the window
propagates back to the webview.

Fix that by entirely bypassing the event propagation code.

Created attachment 339977 [details] [review]
window: use the proper modifier when checking keypresses

We have a handy var which filters out unwanted event modifiers,
but we were using the raw event modifiers.

This shouldn't make any difference, but lets just make the code
clearer.

JFTR, I have tried applying Georges' patches both to my reduced test program
and to the Epiphany sources from Git, and then checked that:

 1. The double-initial letter issue is gone in Facebook, and also in Riot.im
 2. In Google Docs it is still possible to use e.g. Ctrl+B to toggle the
    bold font attribute.

For me they solve the double-capital letter issue (1.) without breaking the
cases (2.) for which the “ephy_window_key_press_event()” function was first
introduced.

Let's please have them applied, *also* in the “gnome-3-22” branch. This solves
a very annoying usability issue, and IMHO it deserves to be in a release, to
not make our users wait 6 months for the fix.

Created attachment 339978 [details] [review]
window: bypass event propagation on composed keys

Same patch adapted to 3.22.

I have also just verified that Georges' patches also solve bug #764654 \o/

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

Review of attachment 339977 [details] [review]:

Thanks a bunch to both of you for working on this.

(Note: I'm reviewing the second patch first, because this one is easier!)

::: src/ephy-window.c
@@ +559,3 @@
+      (modifier & GDK_CONTROL_MASK ||
+       modifier & GDK_MOD1_MASK ||
+       modifier & GDK_SHIFT_MASK)) {

Hm, I think I would write if (event->length > 0 && modifier != 0). All of GDK_CONTROL_MASK, GDK_MOD1_MASK, and GDK_SHIFT_MASK are guaranteed to be part of the modifier mask anyway, and I don't see any particular reason to exclude the super/meta/hyper modifiers from this logic.

Created attachment 339981 [details] [review]
window: use the proper modifier when checking keypresses

Fixed.

Review of attachment 339976 [details] [review]:

Hm, not really satisfied with this. I'll paste our IRC snippets as summary:

aperezdc:  1. in webkitWebViewBaseKeyPressEvent() we do things depending on whether this shouldForwardNextEvent flag is set or not. If not set, TRUE is returned to make GTK believe th event is always handled (and it doesn't bubble up), and the event passed to webkit. When webkit calls doneWithKeyEvent(), it also tells us if it handled the event itself or not, and if it wasn't, we put the event again in the queue with gtk_main_do_event
aperezdc:  which keyses webkitWebViewBaseKeyPressEvent() to be called again, this time with the flag set, which causes the event to be sent to the webview widget, which should bubble it up
aperezdc:  s/keyses/causes
aperezdc:  now at this point, wouldn't it have made sense to inject that event *to the parent widget*
aperezdc:  and not to the webview widget itself?
aperezdc:  becasue, well, it was already tried to be handled by the webview
feaneron:  i'm specially concerned with the "handled = gtk_widget_event (focus_widget, (GdkEvent *)event);" line
aperezdc:  yes, I am positive that the event is sent back to the webview in my simple test app
aperezdc:  which means that gtk sent it first to the web view, the web view sent it to th window, and the window is trying to send it back to th webview

mcatanzaro:  The comment says: /* Pass CTRL+letter characters to the widget */
mcatanzaro:  But the widget (webview) has ALREADY seen these keypresses, and decided to ignore them, right?
mcatanzaro:  In fact, in any case, the widget has already handled any keypress here. Why do we ever need to pass events down to the widget?
feaneron:  only on emacs mode
mcatanzarolooks up event propagation docs, must be missing something
mcatanzaro:  feaneron: Also ESC according to the code
feaneron:  ah indeed
mcatanzaro:  So I'm tempted to just remove this entire function. What besides emacs mode breaks if we get rid of it? (And I'm not convinced that emacs mode does break! We need to check it.)
mcatanzaro:  It looks like our custom handling of ESC just guarantees the webview handles ESC once, then the window handles it, then the webview handles it again, is that right? I mean, is that what is currently happening, because of course it's not correct. :)

Created attachment 339985 [details] [review]
window: fix web view receiving events twice

The current code propagates the event to the web view, then chains up if
the web view doesn't handle the event. But chaining up causes GtkWindow
to propagate the event to the web view yet again. Surely we never want
to do that, so stop doing it.

I think there must be some other bug here, though, in WebKit, that
causes WebKit to sometimes do something with the event but then
propagate anyway, which is wrong. If I'm right, then WebKit is
unfortunately still broken, but this works around it in Epiphany and is
the right thing to do anyway, since sending the same event to the web
view twice is nonsense regardless of whether the web view propagates it
or not.

Review of attachment 339985 [details] [review]:

Looks reasonable to me.

Turns out Georges's patch causes Ctrl+I to open incognito window even in Google Docs, when the desired behavior is to italicize text.

(In reply to Michael Catanzaro from comment #33)
> But chaining up causes GtkWindow
> to propagate the event to the web view yet again.

^ This is the key point from the rest of our discussion tonight.

Code review on this would be great, since this is really sketchy.

Created attachment 339987 [details] [review]
window: fix web view receiving events twice

Comment on attachment 339987 [details] [review]
window: fix web view receiving events twice

Nope, I only tested it on Google Docs. It just crashes when triggering an accelerator that's not handled by the page. Thanks to Georges for finding this.

Summary of our discussion of this:

feaneron:  stack overflow. It's entering an event loop

feaneron:  removing the parent hook fixes it to me

mcatanzaro:  It (GtkWidget's key_press_event) just calls gtk_bindings_activate_event
mcatanzaro:  So I don't see how that causes any event propagation at all

Created attachment 340069 [details] [review]
window: fix web view receiving events twice

The current code propagates the event to the web view, then chains up if
the web view doesn't handle the event. But chaining up causes GtkWindow
to propagate the event to the web view yet again. Surely we never want
to do that, so stop doing it.

I think there must be some other bug here, though, in WebKit, that
causes WebKit to sometimes do something with the event but then
propagate anyway, which is wrong. If I'm right, then WebKit is
unfortunately still broken, but this works around it in Epiphany and is
the right thing to do anyway, since sending the same event to the web
view twice is nonsense regardless of whether the web view propagates it
or not.

Created attachment 340070 [details] [review]
window: fix web view receiving events twice

Created attachment 340085 [details] [review]
window: add blacklist of events to not deliver to web view

Certain window and tab management shortcuts are reserved by Epiphany and
will never be delivered to the webpage, even though webpages should in
general be allowed to override Epiphany shortcuts (e.g. Ctrl+B in Google
Docs should embolden text and not open the old bookmarks dialog, Ctrl+I
should italicize text and not open a new incognito window).

Attachment 340070 [details] pushed as 7506418 - window: fix web view receiving events twice
Attachment 340085 [details] pushed as 7688c7d - window: add blacklist of events to not deliver to web view

JFTR, I am glad this is fixed and committed into “master” now — I've
using the last version of the patches for the last hour without any
loss of functionality, and with the duplicated characters issue not
appearing at all. Nice work!

Is there any change we can get this fixed also in the “gnome-3-22”
branch? It would be good to our users not making them wait for six
months (at least) to get this fix :)

(In reply to Adrian Perez from comment #43)
> Is there any change we can get this fixed also in the “gnome-3-22”
> branch? It would be good to our users not making them wait for six
> months (at least) to get this fix :)

Hopefully, I'm just having a bit of trouble handling ephy_window_bound_accels() properly. Seems like this should work:

{
  EphyWebView *view;

  view = ephy_embed_get_web_view (EPHY_WINDOW (widget)->active_embed);
  if (gtk_window_get_focus (GTK_WINDOW (widget)) != GTK_WIDGET (view))
    return ephy_window_bound_accels (widget, event) ||
           GTK_WIDGET_CLASS (ephy_window_parent_class)->key_press_event (widget, event);

  /* GtkWindow's key press handler first calls gtk_window_activate_key,
   * then gtk_window_propagate_key_event. We want to do the opposite,
   * because we want to give webpages the chance to override most
   * Epiphany shortcuts. For example, Ctrl+I in Google Docs should
   * italicize your text and not open a new incognito window. So:
   * first propagate the event to the web view. Next, try
   * accelerators only if the web view did not handle the event.
   */
  if (!gtk_window_propagate_key_event (GTK_WINDOW (widget), event) &&
      !ephy_window_bound_accels (widget, event))
    gtk_window_activate_key (GTK_WINDOW (widget), event);
  return GDK_EVENT_STOP;
}

but it doesn't work for some reason.

I'm not sure if WebKit is broken or that it can't do better, if you explain the WebKit issue in detail I can look at it. All this mess comes from the fact that GTK+ expects events to be handled synchronously, using the return value of event callbacks to determine whether the event was handled or not to propagate it or not. In WebKit events are handled asynchronously, we don't want to block the UI sending a sync message to the WebProcess for every key press/release. For other kind of events we always claim to handled them (button press/release, scroll, etc.) or to never handle them (motion, crossing). In the case of keyevents, since we don't know in advance if the web process will handle the event we always claim to handle it to avoid propagation, when the web process finishes processing the event we do nothing if it was handled, or inject it again in the gtk event loop if it was not handled, writing down that we shouldn't handle it this time. So, in may of the cases we end up with the same event processed twice bu the gtk event loop. If an application connects to key-press event of the web view will get two callbacks for those key events. This is unfortunate, but I don't see any other way to handle this, maybe we can propose to add support for async event handling in GTK+ 4. A possible workaround is to never connect to WebKitWebView events with the normal flags, but using always g_signal_connect_after, because events are handled in WebKitWebView in the vfuncs.

A possible bug in webkit (or even in the website) that could confuse this, is that the web process tell us that the event was not handled, but the website handled it, and then the web view handles it again.

(In reply to Carlos Garcia Campos from comment #45)
> I'm not sure if WebKit is broken or that it can't do better, if you explain
> the WebKit issue in detail I can look at it. All this mess comes from the
> fact that GTK+ expects events to be handled synchronously, using the return
> value of event callbacks to determine whether the event was handled or not
> to propagate it or not. In WebKit events are handled asynchronously, we
> don't want to block the UI sending a sync message to the WebProcess for
> every key press/release. For other kind of events we always claim to handled
> them (button press/release, scroll, etc.) or to never handle them (motion,
> crossing). In the case of keyevents, since we don't know in advance if the
> web process will handle the event we always claim to handle it to avoid
> propagation, when the web process finishes processing the event we do
> nothing if it was handled, or inject it again in the gtk event loop if it
> was not handled, writing down that we shouldn't handle it this time. So, in
> may of the cases we end up with the same event processed twice bu the gtk
> event loop.

Yes, all of that so far is fine. WebKit is injecting the event a second time for good well-understood reason. In contracts, Epiphany was injecting the event yet again (potentially a third time!) just by accident.

> If an application connects to key-press event of the web view
> will get two callbacks for those key events. This is unfortunate, but I
> don't see any other way to handle this, maybe we can propose to add support
> for async event handling in GTK+ 4.

Hm, this is surely a bug indeed, but Epiphany does not connect to that signal, so it's not the problem here.

Another possible fix would be to add some function to GObject that allow a signal handler to block all subsequent handlers from being called; then WebKit could just install the first handler and deal with that "cleanly". Like g_signal_stop-emission, except it would stop other handlers from being invoked and not just the default method. That wouldn't have to wait for GTK+ 4.

(In reply to Carlos Garcia Campos from comment #46)
> A possible bug in webkit (or even in the website) that could confuse this,
> is that the web process tell us that the event was not handled, but the
> website handled it, and then the web view handles it again.

Yes, this is what I think is happening. Epiphany only double-injected the event in the case the web process does not handle the event. So that means three key-press-events total, at least the first two of which were not handled, yet WebKit (or the website) did something with the event the second time anyway. I suppose it could well be a bug in Facebook and Riot; there's no way for WebKit to prevent that, is there....

(In reply to Michael Catanzaro from comment #47)
> Yes, all of that so far is fine. WebKit is injecting the event a second time
> for good well-understood reason. In contracts, Epiphany was injecting the
> event yet again (potentially a third time!) just by accident.

Er, I meant "in contrast"

(In reply to Adrian Perez from comment #43)
> Is there any change we can get this fixed also in the “gnome-3-22”
> branch? It would be good to our users not making them wait for six
> months (at least) to get this fix :)

Turns out I just made a dumb mistake when trying to backport it. Done now. Tomorrow is release day so you won't have to wait long.

(In reply to Michael Catanzaro from comment #49)
> Tomorrow is release day so you won't have to wait long.

Eh, GNOME 3.22.2 was just two weeks ago... I'll do it with 3.23.3 instead.