Summary: | [GTK] [WK2] Check value of WEBKIT_INJECTED_BUNDLE_PATH | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alberto Garcia <berto> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cgarcia, commit-queue, gustavo, mrobinson | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Alberto Garcia
2013-06-26 05:22:48 PDT
Created attachment 205477 [details]
Patch
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 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?
(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. Comment on attachment 205477 [details] Patch Clearing flags on attachment: 205477 Committed r152084: <http://trac.webkit.org/changeset/152084> All reviewed patches have been landed. Closing bug. |