[GTK] Toggle OverWrite mode when pressing the Insert key

[Sergio Villar Senin]

[GTK] Toggle OverWrite mode when pressing the Insert key

[Sergio Villar Senin]

Created attachment 195258 [details]
Patch

[Martin Robinson]

Comment on attachment 195258 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195258&action=review

> Source/WebCore/platform/gtk/KeyBindingTranslator.cpp:238
> +    if (event->keyval == GDK_Insert && type == KeyDown) {
> +        commandList.append("OverWrite");
> +        return;
> +    }
> +

Why KeyDown and not KeyPress?

[Sergio Villar Senin]

(In reply to comment #2)
> (From update of attachment 195258 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195258&action=review
> 
> > Source/WebCore/platform/gtk/KeyBindingTranslator.cpp:238
> > +    if (event->keyval == GDK_Insert && type == KeyDown) {
> > +        commandList.append("OverWrite");
> > +        return;
> > +    }
> > +
> 
> Why KeyDown and not KeyPress?

We don't apparently get KeyPress for the insert key.

[Martin Robinson]

Comment on attachment 195258 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195258&action=review

>>> Source/WebCore/platform/gtk/KeyBindingTranslator.cpp:238
>>> +
>> 
>> Why KeyDown and not KeyPress?
> 
> We don't apparently get KeyPress for the insert key.

Okay, that's reasonable. Another question is why not simply add this to keyPressEntries? By doing this without respect to modifiers you are breaking Shift+Insert.

[Sergio Villar Senin]

(In reply to comment #4)
> (From update of attachment 195258 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195258&action=review
> 
> >>> Source/WebCore/platform/gtk/KeyBindingTranslator.cpp:238
> >>> +
> >> 
> >> Why KeyDown and not KeyPress?
> > 
> > We don't apparently get KeyPress for the insert key.
> 
> Okay, that's reasonable. Another question is why not simply add this to keyPressEntries? By doing this without respect to modifiers you are breaking Shift+Insert.

I thought about that but the problem with modifiers is that the Insert won't be detected for example if the BloqNum is active (unless we register all the possible combinations). BTW what's shift+insert for?

[Martin Robinson]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 195258 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=195258&action=review
> > 
> > >>> Source/WebCore/platform/gtk/KeyBindingTranslator.cpp:238
> > >>> +
> > >> 
> > >> Why KeyDown and not KeyPress?
> > > 
> > > We don't apparently get KeyPress for the insert key.
> > 
> > Okay, that's reasonable. Another question is why not simply add this to keyPressEntries? By doing this without respect to modifiers you are breaking Shift+Insert.
> 
> I thought about that but the problem with modifiers is that the Insert won't be detected for example if the BloqNum is active (unless we register all the possible combinations). BTW what's shift+insert for?

Is NumLock detected as a modifier? Typically modifiers are keys like Shift and Control that you chord with. Even so, if there is a chord combination that should be active too, you should just add all applicable ones to keyPressEntries. I'm almost certain we don't want to treat Shift+Insert as a switch to overtype mode.

[Martin Robinson]

Looking at this again, instead of adding a special keybinding, you should likely just add your logic to the toggle-overwrite signal! :)

Take a look at toggleOverwriteCallback in that file.

[Sergio Villar Senin]

Created attachment 196860 [details]
Patch

[Martin Robinson]

Comment on attachment 196860 [details]
Patch

This is a lot nicer. :)

[Sergio Villar Senin]

Committed r147916: <http://trac.webkit.org/changeset/147916>