[gnome] firefox should have a search provider
Categories
(Firefox :: Shell Integration, enhancement, P5)
Tracking
()
Tracking | Status | |
---|---|---|
firefox77 | --- | fixed |
People
(Reporter: bytbox, Assigned: stransky)
References
(Depends on 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
The gnome-shell search bar shows aggregated results from a collection of "search providers". It would be nice if firefox included a search provider, so that bookmarks, history, and the option to search with the default engine, showed up in gnome's search bar. AFAIK, epiphany is the only browser to have a search provider. See [1] for the gnome bugtracker discussion explaining why that provider shouldn't just open the default browser. The important part (IMO) is that looking at bookmarks, rss feeds, history, and so forth can't be done in a generic manner. [1] https://bugzilla.gnome.org/show_bug.cgi?id=720245
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
We can utilize the DBusRemoteService here to export search results via DBus. toolkit/components/remote/nsDBusRemoteService.* provides public OpenURL() method which enables open url by running Firefox instance. We may also implement the search provider DBus methods here to provide the search privider cabability.
Assignee | ||
Comment 2•3 years ago
|
||
Looks like we need to implement a dearch provider D-Bus interface described at [1] which should be fairly easy. [1] https://developer.gnome.org/SearchProvider/
Assignee | ||
Comment 3•3 years ago
|
||
Also I think we should provide the search only when Firefox is running so no external application would be needed for that.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 4•1 year ago
|
||
Implement org.gnome.Shell.SearchProvider2 D-Bus interface and enable it when
widget.gnome-search-provider.enabled pref is set, so this feature is disabled
by default.
Assignee | ||
Comment 5•1 year ago
|
||
Assignee | ||
Comment 6•1 year ago
|
||
Depends on D54334
Assignee | ||
Comment 7•1 year ago
|
||
Depends on D54334
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Updated•1 year ago
|
Assignee | ||
Comment 8•1 year ago
|
||
There are some issues which need to be addressed:
- async history search
- correct behavior when there's no result returned from history search
- Fixed Bug 1625850
Assignee | ||
Comment 9•1 year ago
|
||
Implement org.gnome.Shell.SearchProvider2 D-Bus interface and enable it when
widget.gnome-search-provider.enabled pref is set, so this feature is disabled
by default.
Assignee | ||
Comment 10•1 year ago
|
||
Comment 12•1 year ago
|
||
# LOCALIZATION NOTE (gnomeSearchProviderSearch):
# Used for search by Gnome Shell activity screen, %s is a searched string.
gnomeSearchProviderSearch=Search the web for “%s”
I pointed out that %s
is normally not correct. Martin's explanation:
%S does not work. IIUC %S means UTF16 and %s UTF8 encoding and gnome-shell/dbus use UTF8 so we can't use %S here.
There may be confusion here as gnomeSearchProviderSearch string is not used by Firefox but it's send over DBus which expects UTF8 only.
This kind of matches my memory of the issues we had in the past, so it seems wanted.
Having said that, do you expect any problem, at least in our l10n toolchain? I checked and compare-locales
reports an ERROR if locales change from %s
to %S
, which is good. I remember we added checks for printf, no clue if that's relevant here (bug 1388789).
Assignee | ||
Comment 13•1 year ago
|
||
Francesco, I'll remove the %s from the translated string if that causes troubles. It's not necessary to have it there.
Assignee | ||
Comment 14•1 year ago
|
||
Okay, I'll fix the patch to use %S and then I can convert it to UTF8, there's no big deal here.
Comment 15•1 year ago
|
||
(In reply to Martin Stránský [:stransky] from comment #14)
Okay, I'll fix the patch to use %S and then I can convert it to UTF8, there's no big deal here.
I'd wait for Axel to chime in. I might be just overthinking this, and %s
might be OK to use.
Comment 16•1 year ago
|
||
So, we'll need to change the l10n pieces here a bit. printf doesn't equal printf, and caching localized strings is not best practice.
Here's what I'd ask you to do:
Create a string bundle, cache that. Clear the cache on intl:app-locales-changed
, https://firefox-source-docs.mozilla.org/intl/locale.html#events. Use bundle->FormatStringFromName
.
Or cache the string, but also clear that cache on intl:app-locales-changed
, but use nsTextFormatter
.
Ending up in nsTextFormatter
is important, as that's built to not crash on misplaced placeable parameters. And that's important as localizers love to get these wrong.
If you're going through FormatStringFromName
, you'll want to use %S
, if you're using nsTextFormatter
, use whatever is appropriate. You'll find that it doesn't care much, it'll replace the format with the right option for the param you passed in.
Blowing away the cache will get you multi-locale support (more important on linux, too), nsTextFormatter
gives you non-crashyness.
I would have loved to point directly to Fluent, but I don't think that using from C++ is a good idea before we have the localization classes outside of js.
Assignee | ||
Comment 17•1 year ago
|
||
(In reply to Axel Hecht [:Pike] from comment #16)
So, we'll need to change the l10n pieces here a bit. printf doesn't equal printf, and caching localized strings is not best practice.
Here's what I'd ask you to do:
Create a string bundle, cache that. Clear the cache on
intl:app-locales-changed
, https://firefox-source-docs.mozilla.org/intl/locale.html#events. Usebundle->FormatStringFromName
.Or cache the string, but also clear that cache on
intl:app-locales-changed
, but usensTextFormatter
.Ending up in
nsTextFormatter
is important, as that's built to not crash on misplaced placeable parameters. And that's important as localizers love to get these wrong.If you're going through
FormatStringFromName
, you'll want to use%S
, if you're usingnsTextFormatter
, use whatever is appropriate. You'll find that it doesn't care much, it'll replace the format with the right option for the param you passed in.Blowing away the cache will get you multi-locale support (more important on linux, too),
nsTextFormatter
gives you non-crashyness.
Thanks. As I have bundle->FormatStringFromName in place I'll add the intl:app-locales-changed observer to clear the case.
Comment 18•1 year ago
|
||
Pushed by btara@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/cee5051ad409 Implemenet Gnome search provider, r=jhorak,mak
Comment 19•1 year ago
•
|
||
Backed out changeset cee5051ad409 (bug 1239694) for bustages complaining about dbus
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=cee5051ad4093393232950f16d42e50cc66d9c29&searchStr=linux%2Cbuild&tochange=766147895c754341bc633c879a4b7b34f675ce82
Backout link: https://hg.mozilla.org/integration/autoland/rev/766147895c754341bc633c879a4b7b34f675ce82
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299966808&repo=autoland&lineNumber=16015
...
[task 2020-04-29T11:12:59.094Z] 11:12:59 INFO - make[4]: Entering directory '/builds/worker/workspace/obj-build/xpcom/components'
[task 2020-04-29T11:12:59.102Z] 11:12:59 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o StaticComponents.o -c -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -ftrivial-auto-var-init=pattern -DDEBUG=1 -DMOZ_LAYOUT_DEBUGGER -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/xpcom/components -I/builds/worker/workspace/obj-build/xpcom/components -I/builds/worker/workspace/obj-build/xpcom -I/builds/worker/checkouts/gecko/xpcom/base -I/builds/worker/checkouts/gecko/xpcom/build -I/builds/worker/checkouts/gecko/xpcom/ds -I/builds/worker/checkouts/gecko/chrome -I/builds/worker/checkouts/gecko/js/xpconnect/loader -I/builds/worker/checkouts/gecko/layout/build -I/builds/worker/checkouts/gecko/modules/libjar -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -Os -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/checkouts/gecko/widget/gtk/compat-gtk3 -pthread -I/usr/include/gtk-3.0 -I/usr/include/atk-1.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/cairo -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/pixman-1 -I/usr/include/libpng12 -I/usr/include/gtk-3.0/unix-print -fexperimental-new-pass-manager -MD -MP -MF .deps/StaticComponents.o.pp StaticComponents.cpp
[task 2020-04-29T11:12:59.103Z] 11:12:59 INFO - In file included from StaticComponents.cpp:145:
[task 2020-04-29T11:12:59.103Z] 11:12:59 INFO - In file included from /builds/worker/checkouts/gecko/xpcom/components/../../browser/components/shell/nsGNOMEShellService.h:14:
[task 2020-04-29T11:12:59.103Z] 11:12:59 INFO - In file included from /builds/worker/checkouts/gecko/xpcom/components/../../browser/components/shell/nsGNOMEShellSearchProvider.h:11:
[task 2020-04-29T11:12:59.103Z] 11:12:59 INFO - In file included from /builds/worker/workspace/obj-build/dist/include/mozilla/DBusHelpers.h:10:
[task 2020-04-29T11:12:59.103Z] 11:12:59 INFO - /builds/worker/workspace/obj-build/dist/system_wrappers/dbus/dbus.h:3:15: fatal error: 'dbus/dbus.h' file not found
[task 2020-04-29T11:12:59.103Z] 11:12:59 INFO - #include_next <dbus/dbus.h>
[task 2020-04-29T11:12:59.103Z] 11:12:59 INFO - ^~~~~~~~~~~~~
[task 2020-04-29T11:12:59.104Z] 11:12:59 INFO - 1 error generated.
[task 2020-04-29T11:12:59.104Z] 11:12:59 INFO - /builds/worker/checkouts/gecko/config/rules.mk:750: recipe for target 'StaticComponents.o' failed
[task 2020-04-29T11:12:59.104Z] 11:12:59 ERROR - make[4]: *** [StaticComponents.o] Error 1
[task 2020-04-29T11:12:59.104Z] 11:12:59 INFO - make[4]: Leaving directory '/builds/worker/workspace/obj-build/xpcom/components'
[task 2020-04-29T11:12:59.104Z] 11:12:59 INFO - /builds/worker/checkouts/gecko/config/recurse.mk:74: recipe for target 'xpcom/components/target-objects' failed
[task 2020-04-29T11:12:59.104Z] 11:12:59 ERROR - make[3]: *** [xpcom/components/target-objects] Error 2
[task 2020-04-29T11:12:59.104Z] 11:12:59 INFO - make[3]: *** Waiting for unfinished jobs....
[task 2020-04-29T11:12:59.143Z] 11:12:59 INFO - make[4]: Entering directory '/builds/worker/workspace/obj-build/media/libdav1d/asm'
...
Assignee | ||
Comment 20•1 year ago
|
||
Thanks, updated.
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd332797255b8e62c0a454b3c12cef730ea78833
Comment 21•1 year ago
|
||
Pushed by dluca@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/30931766b1b1 Implemenet Gnome search provider, r=jhorak,mak
Comment 22•1 year ago
|
||
bugherder |
Updated•11 months ago
|
Description
•