GTK file picker selection with keyboard or double click is broken

[vannilla]

Since I updated to Pale Moon 34.3.1 the GTK(3) file picker is usable only by using the mouse cursor to press buttons. Keyboard actions and double-clicking produce no effects.
I gave it a spin for a few days in case it was a misconfiguration on my end, but all other GTK applications I have work just fine as they always use to, only Pale Moon is unable to select files through keyboard or double-clicks.

I suspect this commit is the culprit, but I'm not 100% sure as there doesn't seem to be any immediately visible issue with it.
If I switch to the native file picker by toggling the widget.allow-gtk-native-file-chooser preference everything works as expected, but I'd rather use the default value.

[andyprough]

Whether I have widget.allow-gtk-native-file-chooser set to true or false, I get the same file chooser. Is it specific to certain window managers, or to whether you are using the gtk2 or gtk3 version of Pale Moon?

[vannilla]

Whether I have widget.allow-gtk-native-file-chooser set to true or false, I get the same file chooser.

The file chooser should look the same, the differences are mostly behavioural. Can you check that you can select files by pressing enter or double-clicking? Just go to a website (or local file!) with a file input and try it.

Is it specific to certain window managers, or to whether you are using the gtk2 or gtk3 version of Pale Moon?

Shouldn't depend on window managers, especially since other applications do not have this issue, it's purely a matter of behaviour. It happens on GTK2 too, the version is irrelevant.

[andyprough]

Ok, I see you are right. Yes, I get the same behavior - if widget.allow-gtk-native-file-choose is set to false by default, then I cannot double-click or hit the Enter key to choose a file, but if set to true then it works normally. Yes, appears to be a bug.

[Moonchild]

Hmm. Well the ported Mozilla bug is a guard against some social engineering attacks, which is to a degree described in the patch itself (requires tricking the user into weird behaviour to exploit). I'm not sure how else to coerce the dialog boxes on gtk, this seems to be a quirk anyway (why does the ID matter?) And probably needs someone with more knowledge of gtk behaviour to look at it. I just ported this verbatim since it looked ok, as you say as well.

[Moonchild]

Filed Issue #3136 (UXP) to track this.

[ron_1]

If I switch to the native file picker by toggling the widget.allow-gtk-native-file-chooser preference everything works as expected, but I'd rather use the default value.

This workaround doesn't work for me, so I hope this is fixed soon. Can anyone tell me why this fix might not be working for me? And yes, I restarted Pale Moon, still didn't work. Even rebooted computer, still didn't work.

EDIT
So if I switch to the GTK3 version the fix works. Does this info help any? Or does it only have something to do with my computer/OS/Desktop Environment setup?

[JayByrd]

If I switch to the native file picker by toggling the widget.allow-gtk-native-file-chooser preference everything works as expected, but I'd rather use the default value.

This workaround doesn't work for me, ...

Confirmed here for the GTK2 build. I'm spinning up a GTK3 build right now to test...

But while that's compiling in the VM, I've been reading up on some GTK docs, and I'm skeptical about how this "fix" has been implemented.

...this seems to be a quirk anyway (why does the ID matter?) And probably needs someone with more knowledge of gtk behaviour to look at it. I just ported this verbatim since it looked ok, as you say as well.

Indeed. I've been checking the GTK docs, and it seems that, contrary to the description given in the comments of the patch, the numerical value of the "response ID" is not what determines the "default widget" for a given dialog/window.

I am by no means a GTK expert, so take the following with a grain of salt...

Code: Select all

  if (GTK_IS_DIALOG(file_chooser)) {
    gtk_dialog_set_default_response(GTK_DIALOG(file_chooser), GTK_RESPONSE_ACCEPT);
  }

The patch removes this stanza, but never actually sets a new default. Meanwhile, the actions taken in the new "case kFilePickerAccept" seem to be the exact implementation as in the "case GTK_RESPONSE_ACCEPT" stanza that's being removed.

Perhaps I'm oversimplifying, but couldn't one simply change GTK_RESPONSE_ACCEPT to GTK_RESPONSE_CANCEL in the code above? It seems that would mitigate against the "social engineering" trick described, while not creating a new "0" response ID that apparently breaks some functionality. This doc describes all the response IDs.

This describes how the default response is set, and there is no indication that being -3 in value is what makes "accept" the default.

This statement in the comments:

GTK's built-in negative response IDs cause the button to be treated as the dialog's default widget

seems to me to be in error.

[Moonchild]

but couldn't one simply change GTK_RESPONSE_ACCEPT to GTK_RESPONSE_CANCEL in the code above?

I pondered the exact same in the issue I opened. Good to see you had the same idea. The difference would be behavioural for people usually being able to just press enter if the file name is pre-filled. That will now dismiss the dialog in that case. I do think that's an acceptable compromise, though.
What Mozilla tried to do is not have a default at all, meaning it would not respond to input unless first navigating with an access key or clicking, but it doesn't seem like that works.

[Moonchild]

Please let me know if this PR solves the issues:
https://repo.palemoon.org/MoonchildProd ... pulls/3137

[vannilla]

Just to make sure nothing is overlooked, my normal workflow is:

1. picker opens;
2. if the file is already selected press enter;
3. otherwise look for it and select it with a single click or arrow keys;
4. confirm with enter or double click, depending on which is more comfortable at that moment.

If fixing this issue means point 2 does not work anymore I am fine with it, as long as I get enter/double click back.

[Moonchild]

Correct, point 2 wouldn't work any more because that would be the very thing being abused in the attack.

[JayByrd]

Please let me know if this PR solves the issues:
https://repo.palemoon.org/MoonchildProd ... pulls/3137

Apparently, I did oversimplify it--GTK quirk, indeed!

OK, the double-click and -ENTER- key functionality is restored by the changes in your PR. However, it seems that "accept" is still the default!?!

Tested with this mp3 file:
When the file chooser opens and I press -ENTER-, the file download begins immediately with the default file name. When I press the key, the "Save" button in the chooser window visually shows as having been pressed! How can this be after changing the default to "cancel"?

What I suspect is that the disconnect here could be the subtle difference between the "default response" and the "default widget." Apparently, the default widget is the text entry field, not the buttons..? Thus, it could be that we're setting a default for a cancel button, but the button isn't "in focus" from the keyboard's perspective, the text entry field is. When GTK recognizes an -ENTER- from a text-entry field, it just accepts it. (Which does seem to be the logical thing. I mean, you don't want to type in a custom file name, press -ENTER- and it cancels!)

If my suspicions here are correct, then to successfully neutralize the social engineering exploit, we'd need to change the default "focus" to the buttons.

The downside being that I'm very accustomed to the practice of immediately typing in my own file name and pressing -ENTER-. If the buttons are the immediate focus, one would need to use the mouse or -TAB- key to move to the text entry field. An annoyance to be sure, but not a deal breaker I suppose.

[Moonchild]

I see. Well, I'm just not familiar with gtk common dialogs enough to fix this, then. But I'm not even sure it's that pressing (read on...)
Ultimately, we can also just leave it as it was. The exploit is asking the user to do something really weird (I won't elaborate further since it's not a public bug) that should set off alarm bells for anyone having been on the web for any length of time and being aware of the hostile nature of dodgy sites, on top of requiring the browser to already be in a very specific state. Considering the more savvy nature of Linux desktop users, it'd be a lot less likely to be a valid exploit that would work in the wild, even if the browser is in that required state. Even in Mozilla land, it's marked low severity because of what is needed to make this work, so while it does have a CVE, the fix would actually be an optional improvement.

[JayByrd]

I see. Well, I'm just not familiar with gtk common dialogs enough to fix this, then.

Me neither, but I'll probably look into the GTK docs a little further and see what I can learn... Though, surely there is someone around who is more familiar? Didn't I see a thread, or perhaps multiple threads, about GTK2 revival efforts? I'm just an amateur sleuth, not a coder...

But I'm not even sure it's that pressing (read on...)
Ultimately, we can also just leave it as it was. The exploit is asking the user to do something really weird (I won't elaborate further since it's not a public bug) that should set off alarm bells for anyone having been on the web for any length of time and being aware of the hostile nature of dodgy sites, on top of requiring the browser to already be in a very specific state. Considering the more savvy nature of Linux desktop users, it'd be a lot less likely to be a valid exploit that would work in the wild, even if the browser is in that required state. Even in Mozilla land, it's marked low severity because of what is needed to make this work, so while it does have a CVE, the fix would actually be an optional improvement.

I agree entirely.

[JayByrd]

Looking at this further, it really does seem like the PR should work as intended! From the GTK docs linked to above:

Gtk -> Dialog -> set_default_response
Description

Sets the last widget in the dialog’s action area with the given response_id as the default widget for the dialog. Pressing “Enter” normally activates the default widget.

Right? So using "gtk_dialog_set_default_response" should be the way to go, moving the focus to "cancel" instead of the text entry--but no...

[dinosaur]

Since I updated to Pale Moon 34.3.1 the GTK(3) file picker is usable

Working fine for me with GTK2 builds, but I don't even use it: I have ui.allow_plateform_file_picker set to false in about:config (I prefer PM's file picker to GTK2's: leaner, less UI clutter).

[Moonchild]

I've just backed out the change, reverting to what we had before, instead of leaving the default set to something that isn't working as expected, to avoid other side effects.
I'm on the fence about making an out of band release for this (for Linux only) but I'll let the vocal minority of Linux users decide whether they think it should be done or not.

[andyprough]

I'm on the fence about making an out of band release for this (for Linux only) but I'll let the vocal minority of Linux users decide whether they think it should be done or not.

No need, we have the workaround that works perfectly fine for now.

[vannilla]

I trip on it because of muscle memory but I don't really have any strong feelings one way or another, as long as it gets addressed (it was addressed).