Summary: | [WPE] Install files needed for WebKitWebExtensions | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrian Perez <aperez> | ||||||||||
Component: | WPE WebKit | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bugs-noreply, commit-queue, mcatanzaro, zan | ||||||||||
Priority: | P2 | ||||||||||||
Version: | Other | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://github.com/Igalia/dinghy/pull/13 | ||||||||||||
Bug Depends on: | 180608 | ||||||||||||
Bug Blocks: | 178894 | ||||||||||||
Attachments: |
|
Description
Adrian Perez
2017-11-21 05:01:13 PST
*** Bug 179956 has been marked as a duplicate of this bug. *** I told Adrian a while ago that I would do this (it should be very easy), but after working on bug #173915, I'm no longer sure if we should do it at all. The problem is that there is really not all that much you can do with the web process API if the entire DOM API is unavailable. Almost everything we do in Epiphany's web extension depends on the DOM API. The only remaining API I can think of that's really useful and doesn't is WebKitWebPage::send-request. Maybe that in itself is useful enough to justify exposing the web process API, but I don't think so. Also, whatever we expose now will be redundant with a future JavaScript API. For example, say you want to use WebKitWebPage::form-controls-associated or the new WebKitWebPage::will-submit-form. Both of those depend on the DOM API, but they aren't themselves part of the DOM API. That means our JS API will need to contain bindings for the entire web process API (WebKitWebPage et. al.), not just the DOM API, in order to be a real replacement for the current WebKitGTK+ API. So I'm not sure what we should do. We should discuss with Carlos Garcia. (In reply to Michael Catanzaro from comment #2) > So I'm not sure what we should do. We should discuss with Carlos Garcia. I'm going to WONTFIX this for now. We can decide at any point in the future that we want to expose this API, or not. (In reply to Michael Catanzaro from comment #3) > I'm going to WONTFIX this for now. We can decide at any point in the future > that we want to expose this API, or not. We decided this should be exposed. This is done in trunk, but as part of a huge JSC GObject API commit, so it will require a bit of effort to extricate the relevant bits and backport to 2.20. Actually, thinking this through more, we don't really need it in 2.20: there is only one useful function (send-request) without the DOM or JSC APIs. So I think it's good enough to have this in trunk. (In reply to Adrian Perez from comment #0) > Currently, this API is unusable in the WPE because the following are missing: > > - A “wpe-webkit-webextension.pc” (or similarly named) file for pkg-config. > - API headers (“WebKitWebExtension.h” et al.) I was mistaken; all of this is still missing. Reopening. The injected bundle itself is not installed, so more than development files is needed here. Created attachment 337433 [details]
Patch
Comment on attachment 337433 [details] Patch I think this needs updating to reflect changes in bug #184376. Yup, and also to add API versioning Created attachment 337801 [details]
Patch
Created attachment 337802 [details]
Patch
Comment on attachment 337802 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337802&action=review > Source/WebKit/PlatformWPE.cmake:336 > + DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/wpe-${WPE_API_VERSION}/WPE/wpe" Did we not drop the extra uppercase WPE component in this path? Is the path even correct? It's not correct, good catch Comment on attachment 337802 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337802&action=review >> Source/WebKit/PlatformWPE.cmake:336 >> + DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/wpe-${WPE_API_VERSION}/WPE/wpe" > > Did we not drop the extra uppercase WPE component in this path? Is the path even correct? Ah, here's the problem, the include path commit had not yet gone in when I rebased this patch today, and I only accounted for that in the pkg-config file, but not here. Created attachment 337809 [details]
Patch
BTW, I am still thinking that we can just have this in trunk and not backport it to 2.20. It's not really needed there. Comment on attachment 337809 [details] Patch Clearing flags on attachment: 337809 Committed r230661: <https://trac.webkit.org/changeset/230661> All reviewed patches have been landed. Closing bug. |