Bug 118055 – [GTK] [WK2] Check value of WEBKIT_INJECTED_BUNDLE_PATH

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Alberto Garcia]

If the WEBKIT_INJECTED_BUNDLE_PATH if exists, then the injected bundle
path is taken from there and the system directory is completely
ignored. This is a problem if the custom directory does not exist.

The MiniBrowser for example has WEBKIT_INJECTED_BUNDLE_PATH hardcoded
to $(top_builddir)/.libs. This means that it cannot be distributed
since it won't work outside the build tree (unless we manually
override that variable).

Although maybe being distributable is not the purpose of the
MiniBrowser, there's also no reason not to make it work if webkit is
properly installed in the system.

One solution is to fall back to the system directory if the other one
does not exist, which is also what we do with the web inspector in
WebInspectorProxyGtk.cpp.

[Alberto Garcia]

Created attachment 205477 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Carlos Garcia Campos]

Comment on attachment 205477 [details]
Patch

I think providing an invalid directory in WEBKIT_INJECTED_BUNDLE_PATH is an error of the one who set the variable. This env var is only useful for internal tests and tools, users of the WebKit2GTK+ API should use web extensions instead. So, I'm not sure it's a good idea to add a blocking I/O call here. On the other hand, the check is only done when the env var contains something (this shouldn't happen in most of the cases), so I guess it's harmless. I wonder whether we should return NULL and/or print an error in stderr in that case instead of falling back to system directory. It can be very confusing if I make a typographic error and the installed injected bundle is used silently. What do you think?

[Alberto Garcia]

(In reply to comment #3)
> I think providing an invalid directory in
> WEBKIT_INJECTED_BUNDLE_PATH is an error of the one who set the
> variable.

I was wondering how to make MiniBrowser work outside the build tree. I
know it's just an internal tool, but I guess it still can be useful
(of course if a distributor wants to ship it they can easily remove
the hardcoded WEBKIT_INJECTED_BUNDLE_PATH value).

It's worth noting that the behavior in the case of
WEBKIT_INSPECTOR_PATH is different (we silently fall back to the
system directory if the other one doesn't exist). So I'd say we should
make them the same for the sake of consistency at least.

I don't have a strong opinion in this case since it's really not so
important, but printing and message to stderr sounds good to me.

[WebKit Commit Bot]

Comment on attachment 205477 [details]
Patch

Clearing flags on attachment: 205477

Committed r152084: <http://trac.webkit.org/changeset/152084>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.