1284391 - Remove ui.allow_platform_file_picker pref on GTK widget

Status: RESOLVED FIXED

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

Description

[Makoto Kato [:m_kato]]

ui.allow_platform_file_picker pref changes that file picker uses XUL version instead of GTK version (reason is bug 333653).  But now, we use GTK3 (and we require GTK2.18 even if GTK2 build), so we can remove this option.

Comment 1

[Karl Tomlinson (:karlt)]

I'm not sure that removing this is a good idea.  The GTK file picker has some very undesirable features and I suspect many people prefer to turn it off.

Comment 2

[CoJaBo]

I feel like I should specifically call attention to just how important the Bug 1268395 dependency is. Firefox is the default browser in Kubuntu 14.04 LTS/Trusty Tahr, which (as a long-term support release) still has a significant userbase and will be supported until 2019. I've also heard reports that several other distros use similar versions of KDE and GTK libraries.

On these operating systems, it is not possible to upload or download files without manually setting ui.allow_platform_file_picker to false. It's already bad enough that the default browser is so broken in its default configuration, but removing this pref will remove the only possible workaround; since uploading and downloading files is kind of an important thing to do, implementing this change without due care will inevitably force yet another large group of people to switch to Chrome.

I have personally had to switch an office of dozens of people to Chrome back when Firefox shipped several Windows major-version builds without working bookmarks. I watched the Android Market ratings drop from near perfect to garbage when Firefox Mobile shipped the native branch months before hapatic feedback and long-press worked on it. Let's try to avoid shipping a build without upload/download functionality, alright?

Comment 3

[:Paolo Amadini]

Karl, I'd like to get a sense of comment 1 and the bug mentioned in comment 2, and if there is anything we can do about them.

Code from the XUL file picker turned up in searches related to XBL removal. This reminds me that this particular part of the code looks insufficiently tested, and not something we would like to maintain or convert to HTML once we remove XUL.

Bug 1268395 seems pretty bad but it may be outdated now, and it also lists other workarounds. I'd also say that if "it is not possible to upload or download files without manually setting ui.allow_platform_file_picker to false", we're pretty broken on this platform anyways and we should find a different solution that works right after install, not require users to switch an obscure preference.

What's your take on the subject?

Comment 4

[Karl Tomlinson (:karlt)]

Once a user knows the easter egg for bug 1285795, they can learn to use the
GTK dialog, even if a number of things are also not intuitive.
I've listed some issues that I could recall below.
I don't use the xul picker and so I don't know whether or not it is better.

My take is, if the xul picker is going to take effort to maintain, then remove
it.  It would make more sense to spend effort on improving the GTK dialog than
on maintaining the XUL dialog.  I'm guessing the XUL picker still uses a
nested event loop.  If so, then that's an excellent reason to remove it.

Bug 1268395 is sad, and some people may still be affected, but that is a bug
in the GTK theme, and it is the responsibility of distributions supporting old
packages to fix awful bugs like that.

The primary selection is overridden by the save dialog even when selection is
not performed by the user.

The default text entry selection looks focused even when it is not
https://bugzilla.gnome.org/show_bug.cgi?id=746202 was filed for that and was
marked fixed.  https://bugzilla.gnome.org/show_bug.cgi?id=770941 was filed
later.
These are related:
https://bugzilla.gnome.org/show_bug.cgi?id=596841
https://bugzilla.gnome.org/show_bug.cgi?id=784778
https://bugzilla.gnome.org/show_bug.cgi?id=748672

Other focus issues including
https://bugzilla.gnome.org/show_bug.cgi?id=765551

The incremental search sometimes hangs the whole browser for 10s of seconds.

The dialog is confusing to use without a mouse because tab doesn't move focus
from the text field.

It is tricky to paste locations.
https://bugzilla.gnome.org/show_bug.cgi?id=422485

Comment 5

[:Paolo Amadini]

(In reply to Karl Tomlinson (:karlt) from comment #4)
> My take is, if the xul picker is going to take effort to maintain, then
> remove it.  It would make more sense to spend effort on improving the GTK dialog
> than on maintaining the XUL dialog.  I'm guessing the XUL picker still uses a
> nested event loop.  If so, then that's an excellent reason to remove it.

Yes, this is going to take quite some effort to maintain as we are removing most of the features it currently uses from the platform, including XBL bindings and XUL trees.

Thanks for listing the issues with the platform dialog, it's a good reference. I agree that these should be considered as fixes for the benefit of all applications, rather than us trying to support an obsolete part of the code as a workaround, which may also make things worse by diverting effort and attention from the more important platform fixes.

Comment 6

[:Paolo Amadini]

Attachment: Bug 1284391 - Remove ui.allow_platform_file_picker pref on GTK widget.

Review commit: https://reviewboard.mozilla.org/r/211730/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/211730/

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8941425 [details]
Bug 1284391 - Remove ui.allow_platform_file_picker pref on GTK widget.

https://reviewboard.mozilla.org/r/211730/#review217658

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b8a5f9aac6f
Remove ui.allow_platform_file_picker pref on GTK widget. r=karlt

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7b8a5f9aac6f

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/22778ea407d2
Port bug 1284391 to TB/IB/SM: remove filepicker.xpt and nsFilePicker.manifest/js from package manifests. rs=bustage-fix