WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
118055
[GTK] [WK2] Check value of WEBKIT_INJECTED_BUNDLE_PATH
https://bugs.webkit.org/show_bug.cgi?id=118055
Summary
[GTK] [WK2] Check value of WEBKIT_INJECTED_BUNDLE_PATH
Alberto Garcia
Reported
2013-06-26 05:22:48 PDT
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.
Attachments
Patch
(1.56 KB, patch)
2013-06-26 05:31 PDT
,
Alberto Garcia
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alberto Garcia
Comment 1
2013-06-26 05:31:49 PDT
Created
attachment 205477
[details]
Patch
WebKit Commit Bot
Comment 2
2013-06-26 05:33:03 PDT
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 3
2013-06-26 06:34:19 PDT
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
Comment 4
2013-06-27 01:09:06 PDT
(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 5
2013-06-27 01:49:50 PDT
Comment on
attachment 205477
[details]
Patch Clearing flags on attachment: 205477 Committed
r152084
: <
http://trac.webkit.org/changeset/152084
>
WebKit Commit Bot
Comment 6
2013-06-27 01:49:52 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug