We've run into issues attempting to use JavaScript aggregation/compression with JW Player. After reviewing the module, it's noted that a to-do is to move toward the use of #attached
calls for JS and Drupal.behaviors
for building players.
We have created a patch that addresses these issues, and also puts 7.x-2.x more in alignment with the work being done for 8.x (https://www.drupal.org/node/2712615).
A major component in this patch is shifting away from using three separate formatters. As is described for JW Player 7, all styling differences are now set through CSS, and options for any type of player can be generated in a single array (see the function jw_player_build_player
in our patch). For reference:
https://developer.jwplayer.com/jw-player/docs/developer-guide/customizat...
We're certainly open to discussion, but I'm not sure I see a need for supporting three different formatters, when we were able to construct code that provides all possible options.
The patch also includes support for text and link_field. These are needed by our client to post YouTube videos into JW Player. All players can be modified by developers using the hook_jw_player_build_player_alter
function.
Patch will be posted in a moment for review.
Comment | File | Size | Author |
---|---|---|---|
#31 | jw_player-refactor_theming-2713725-31.patch | 57.92 KB | ron_s |
#31 | interdiff-2713725-30-31.txt | 5.08 KB | ron_s |
#30 | jw_player-refactor_theming-2713725-30.patch | 57.19 KB | ron_s |
#30 | interdiff-2713725-28-30.txt | 4.37 KB | ron_s |
#29 | jw_player-player_type.png | 23.27 KB | ron_s |
Comments
Comment #2
ron_s CreditAttribution: ron_s commentedAttached is a patch for review. The approach we've used is similar to the structure of the Leaflet module. We're open to modifying the approach if you believe some changes are necessary to more closely match 8.x.
Also important to note, you may get a warning when applying this patch, because of the changes we're attempting to make in multiple patches. However based on our review, this patch is only a warning and should be applied correctly.
To ensure all functionality works as expected, we'd highly recommend adding a few other patches we have recently created to this one:
Comment #3
ron_s CreditAttribution: ron_s commentedAdding a link to this thread for 8.x (https://www.drupal.org/node/2645242), since this patch includes an option for the
link_field
.We'll want to make sure this patch includes any necessary link validation checking that we might have missed.
Comment #4
ron_s CreditAttribution: ron_s commentedWorthwhile to mention that this patch and the preset sources patch (https://www.drupal.org/node/2713701) introduce two new JavaScript files that have been put into a new "js" directory.
I would have liked to also move the
jw_player_seek.js
file into this directory to be consistent, but decided to leave it out for now. If we agree to start using the js directory, we can create another patch after the commits are done to movejw_player_seek.js
.Comment #5
BerdirYour structure is a bit different to what I did in the 8.x branch, I have a players list where the key is the html_id.
I have no clue about JS really, so if this is better, then we can change that there.
I was thinking about a similar hook in the 8.x issue too. But with a #type, people could also just alter that and add another pre_render callback. A bit more complicated.
Probably needs a bit more documentation about what $build exactly is (is it one player now or multiple) ?
Yeah, so this is a BC problem.
#theme = 'jw_player' was kind of an API (because it was the only way to do it outside of fields). Removing this will break all those existing places.
That's.. a problem. In 8.x, the same is happening, but there are much, much less sites and the beta is pretty fresh, so I can live with breaking that.
We still have the template. What if we support both? looks like the build function actually just uses an HTML tag right now? I think it would make sense to keep a template, people might want to have some standard markup/classes and overriding a template is easier for frontend people than implementing a render array alter hook.
Either way, the preprocess function could detect if it gets the build settings or the old stuff, and if everyting is prepared already, it could just bail out.
Problem is that it's too late to use #attached I think at that point in 7.x
Comment #6
ron_s CreditAttribution: ron_s commentedI'm assuming I should be looking at this patch? https://www.drupal.org/files/issues/jw-player-type-js-2712615-2.patch
Yes, we're just creating a simple
$settings
array, since each$build
is it's own player:As I mentioned in #2, we liked the approach that the Leaflet module took to structuring this code, and leveraged some concepts (http://cgit.drupalcode.org/leaflet/tree/leaflet.module?id=e505e9c#n90). There are a lot of similarities in the basic principles of both modules, including the idea of creating a single function that can be called to build the end result: http://cgit.drupalcode.org/leaflet/tree/README.txt?id=e505e9c#n25 Their "features" is basically our "options".
If users want to create a JW Player in code and render it to the page, they should be able to do so. It doesn't need to be tied to any particular field or file to accomplish this. In fact, it would allow developers to create wide-ranging solutions, including pulling files from different fields or feeds into one playlist. I would think that keeping the
html_id
out of the associative array would help in creatingjw_player_build_player
elements as easily as possible, but I'm certainly open to discussing further.Yes, maybe look a bit at those Leaflet functions. There are probably even some improvements that can be made to the patch I posted, but I'm not sure we're ready to have JW Player make that big of a change. If you think we can take it further and support a full build function, I'm open to suggestions.
Yes, more documentation is always good. JW Player has different associative array styles depending on if it is a single player, playlist, or playlist with fallback sources. If the array is structured in the right way and passed through
$build
, JW Player will render it. The information they have in Leaflet is a good starting point for what could be done:http://cgit.drupalcode.org/leaflet/tree/README.txt?id=e505e9c#n25
http://cgit.drupalcode.org/leaflet/tree/leaflet.api.php?id=e505e9c
What about trying to conditionalize the code? Add an option on the general settings page such as...
[x] Legacy theming: uses traditional templating and may have JavaScript aggregation/compression issues.
[ ] Improved theming: hook available to change player options, function to build players in code, and best approach for caching and aggregation.
Note: if changing from Legacy to Improved theming and templates were previously used, updates may be necessary.
Haven't entirely thought through if this could work, but wanted to mention as an idea.
Comment #7
ron_s CreditAttribution: ron_s commentedMaking a note that we've identified a minor bug. The following code:
Should instead be this:
Waiting for further feedback before creating any new patches.
Comment #8
ron_s CreditAttribution: ron_s commentedWe've tested out the idea in #6 of legacy vs. improved theming, and I think this is going to work perfectly. We keep all the same code as exists today in 7.x-2.x, and wrap it with a
jw_player_use_legacy_theming
function.Once we get the other patches committed, we'll create a new patch for the theming. Thanks.
Comment #9
ron_s CreditAttribution: ron_s commentedHere is an image of how the approach looks on the settings page.
Comment #10
deanflory CreditAttribution: deanflory commentedI just got this when trying to apply jw_player-refactor_theming-2713725-2.patch to jw_player-7.x-2.x-dev:
And, as I was successful in applying jw_player-preset_sharing_mute_validate-2713701-2.patch, this was the resulting error after running update.php:
So, I guess I'll have to hold off until the refactor_theming patch is worked-out. Thanks for all the effort!
Comment #11
ron_s CreditAttribution: ron_s commented@deanflory, sorry we've been making some adjustments in the past few days. Also the #2 patch on this thread no longer matches the patch on Issue #2713701.
Waiting for some feedback from @Berdir, but we already have a new patch ready for this thread once we can get the other patch committed. Hopefully we'll have it ready soon. Thanks!
(I should also mention it sounds like
jw_player.theme.inc
was probably put at the root of your site in a /theme folder. Depending on how you apply patches, that can happen when adding new files. Check to see if you have a file located there.)Comment #12
deanflory CreditAttribution: deanflory commentedI apply patches in a completely separate folder outside of my local root, then copy the patched version to the server, run update.php, if that works with no problems then I copy it into my local root. It wasn't there because I reverted back to a version that didn't have that theme patch applied, and was why there wasn't a jw_player.theme.inc to be found (was there on my test where I applied both patches and one failed and failed many times, but I don't use or upload if anything fails).
FYI, when I applied both patches jw_player.theme.inc was created at the module folder level:
/.../sites/all/modules/jw_player/jw_player.theme.inc
but the error is looking for it in the theme folder:
/.../sites/all/modules/jw_player/theme/jw_player.theme.inc.
Comment #13
ron_s CreditAttribution: ron_s commentedOk, this is really starting to look good. We've got a defined array structure for a theming API, and able to create custom players on demand directly in code.
Here is a playlist I just created for a sidebar block. Very straightforward, and have written up all the documentation on how to do this in the README file.
All the functionality is outside of the existing theming, so it's 100% backwards compatible. Look forward to getting this out there for review!
Comment #14
ron_s CreditAttribution: ron_s commentedAlso let me know if you think it's easier for developers if the id passed in is part of the data array, or outside of it. For example, could have:
Where
$player_id
is pulled out of the array, or:As it is structured in comment #13. I'm fine with either, but not sure if some people might find it easier with an array that is one level smaller.
Comment #15
ron_s CreditAttribution: ron_s commentedOne other comment worth mentioning... because the API matches directly to JW Player's configuration API, it's possible to add functionality that doesn't exist in the module UI.
For example, I was able to add a logo to my player just by adding the following to the API
'options'
array:Even though this is not available on the site's front-end, developers now have the ability to tailor the player to meet their specific requirements.
If someone needs advertising, captions, related videos, or any other feature not included in the module, it can be added to the array in code.
Comment #16
BerdirSounds like you get closer to my approach in 8.x.
The main difference is that I'm using a render element instead of a builder function. If you have an array as API, you'd just need to use #something instead of just 'something' and additionally #type jw_player. The logic woud be executed as a #pre_render callback registered in hook_element_info().
About the example above, I understand that options are what's passed to jw player (I called that settings in my patch, but options might make more sense if that's what JWPlayer uses.). But what is settings exactly? Seems like the preset could be a top level key, seems important enough.
Comment #17
ron_s CreditAttribution: ron_s commentedThe attached patch uses
#type = 'jw_player'
and a#pre_render
callback. I think this should meet all of the possible needs.Please see README.txt and jw_player.api.php for full information on API usage. Thanks.
Comment #18
ron_s CreditAttribution: ron_s commentedWorth mentioning that we've moved all the "build" functions into an include file called jw_player.build.inc, and placed it in an "includes" directory.
Once the rest of this functionality is approved and committed, we plan to reorganize functions into the following includes:
includes/jw_player.admin.inc (moved from module folder)
includes/jw_player.build.inc (included in this patch)
includes/jw_player.formatters.inc (all formatter functions)
includes/jw_player.legacy.inc (functions related to legacy theming)
includes/jw_player.library.inc (all library functions)
This should cleanup jw_player.module quite a bit, and have it focused on main functions and the #pre_render process.
Comment #19
johnchqueWhite spaces. :)
Comment #20
ron_s CreditAttribution: ron_s commentedFixed the white spaces, and a few other minor items.
Comment #21
ron_s CreditAttribution: ron_s commentedA couple of minor adjustments.
For sites that have not saved with the new preset options, there will be an error when attempting to view the field formatters. Values such as
$preset_settings['mute']
will not be defined, therefore we need someisset
conditionals.Also realized while doing some testing that the
$preset_settings['skin']
value needs to be modified to an array. Otherwise those hooking the player won't be able to change settings like skin background color without initially converting the value to an array on their own.Comment #22
ron_s CreditAttribution: ron_s commentedJust ran into a major issue.
#pre_render
causes File entity types to throw errors and content does not load. We did not see this problem with the original approach.Feedback?
Comment #23
ron_s CreditAttribution: ron_s commentedWe found the issue.
#file
is used by File entity types, and if we attempt to use #file in the JW Player element, it's going to cause serious issues. Changing this to another name fixes the issue.Let me know if you prefer something like
#files
or#jwplayer_files
. We'll create a new patch with#files
for now.Comment #24
ron_s CreditAttribution: ron_s commentedHere is the update. Changes all references in the
hook_element_info
approach from#file
to#files
to ensure no conflicts with File Entities.Comment #25
BerdirFirst, pretty long review of the new approach below. Make sure you read everything before starting to do something or responding, I changed my opinion sometimes while learning more about the patch. We might also want discuss this in IRC or so before you continue to work
Legacy and improved is pretty vague.
Why not name it after what it actually is: "Inline Javascript" ? In 7.x-1.x, there is a similar checkbox but I think pretty broken.
Not sure what to name improved then, but if we make the UI a checkbox, we might not need a name for it, at last not in the UI.
Also, I'd actually recommend to defaut to the new way and add an update function for existing sites that just sets it to the old way with a message that recommends to switch.
Drupal 7 actually not in all places yet, but i would actually recommend to just return the render array in most cases. Maybe just leave out the first sentence here.
I still don't understand why you need that complexity.
In the end, you still hash it, so I don't see how it helps anyone to be predictable.
Also note that in 8.x, I'm only generating a html_id if it wasn't explicitly set. So that in case someone wants a fixed id, because it is guaranteed to be unique, he can do it.
Coding standards require that..
* all arguments are documented
* return is documented if applicable
* empty line between initial line and additional documentation.
(applies to many build functions)
Function name and description is also pretty vague. The description doesn't tell me anything that's not already in the function name. What does "build file" even mean? Wouldn't jw_player_create_file_url() make more sense, since that's essentially what you are doing?
Per my notes above, the second option is imho always "recommended", we just keep support for legacy/inline to make upgrading for existing sites easier.
As mentioned above, this is standard drupal functionality that we shouldn't have to document. Especially since you shouldn't have to do it in 90% of the cases.
This doesn't make sense. new installations do not run update functions, only existing sites do. So for those, you want to make sure the behavior doesn't change for them.
I personally don't like moving code to include files when they can't be autoloaded. It's just another file that has to be loaded on every request. Doesn't actually help performance in any way.
hm, not sure I like dynamically registering a different template. Seems to make it harder for sites to convert.
#type doesn't need to be specified, that's already there. These are just the defaults that are merged into the render array that's being displayed.
don't get the difference between those two yet, but maybe I'll get to it later.
This is also problematic and IMHO makes switching harder than it has to.
Just like you had t before, I would just keep all the existing formatters, possibly update the label to include (deprecated). The method might not even care about which one was selected and just do always the same thing. Probably the settings too, assuming that the settings themself are backwards compatible.
!empty() does the same as isset() + checking for the value.
I think I would still prefer an approach where the primary and practically only difference between the legacy/improved setting is this. And that we support both in the API.
As suggested above, I could see that working by simply checking the provided render array in preprocess. If it has a certain key/structure, preprocess is a no-op and doesn't define any inline js things. If not, it does what it did before.
I also see that legacy vs improved theming does much than just inline js or not, with the player type and so on. So my suggestions above might not make too much sense.
one detail that I noticed is that you in almost every place do a reverse use legacy theming check. Might be easier to read to have a function that returns TRUE for non-legacy instead?
This is an isset() check, but your element info actually sets #preset to an empty string. That means this will always be TRUE. Use !empty() or do not define it as a default key in element_info()
Ah, I see.
Again, hook_element_info() is to define defaults. And you should try to support explicitly setting those values upfront and only automatically set them if empty.
Also, once more, I don't understand the point of the alter hook, if it's a hash based on the file ID and other things. How could anyone guess a correct alter hook name for this?
The only useful alter hook pattern that I can see is one that would be based on the preset.
Note that hook implementations are cached. By defining highly dynamic and unique hooks, you are constantly invalidating and polluting that cache item, which loaded on every request.
it works due to how we merge, but i find this a bit a weird way to specify the settings (without an explicit, unique key). You have the key inside, while I had it as the key. Still not sure what's the better way.
libraries can have dependencies I think in Drupal 7 too. So jw_player_drupal could depend on jw_player and you don't have to specify it twice.
Also, where do per-preset cloud players come into play now, can't see that yet?
We could add it inline, but then we need to decide what to do about the licence key. I'm not sure what should happen exactly when you have the license key globally and select a cloud preset?
For Drupal 8, since it's not possible to reference JS files directly, there's only one way to do this:
Instead of a single jw_player library, we will have to register one for each preset. And then there either use the local player with license key or the given cloud URL. And possibly a generic one as well if no preset is specified.
Then we add the correct library here. The dependency suggestion from above will then no longer work.
Since 7.x already partially uses libraries, that might in fact be the only way that makes sense here as well? Just dynamically adding the preset-cloud url at this point will also clash with the default cloud URL.
as written above, I imagine this would then go away. Even right now, this isn't necessary as you have a separate template currently.
if we do my suggestion above, then this would change to some check to check if this is coming from the #type or not.
I would also use an early return here, simply to avoid indendating everything in this function, hard to see if anything changed or not.
not sure if this file just moved or was changed. not sure if you set up git diff for renames or if there are some changes and it's a small file, so doesn't detect as a rename.
Might be better to keep that out of the patch, adds some noise to a really big patch.
still somewhat surprised that we're not even printing the video tags anymore, is that the recommended way now? I guess it could be faster for rendering, if the browser doesn't try to display the native video player before loading jw player?
That said, didn't you say that jw player removes all classes from the main div? Should we consider to add a wrapper diff with some useful classes instead then?
Comment #26
ron_s CreditAttribution: ron_s commentedI think I follow your thought process fairly clearly. There are some items we'll need to discuss further. The few patches I've recently posted I believe are:
1) tasks outside the scope of the core refactor theming issues
2) not particularly relevant to comments you've made
3) items that I'm hoping we can get committed relatively easily so we are able to focus on issues that need actual attention
Comment #27
ron_s CreditAttribution: ron_s commentedResponding to your comments, let me start by mentioning the ones where I think we can agree, with exceptions/comments in parentheses.
Agree:
#1 (See "Use of #player_type" below for further discussion.)
#2
#4
#6
#7 (Error, needs to be in
hook_install
)#10 (
#type
should have been previously removed)#13 (Already fixed in https://www.drupal.org/node/2719995)
#15 (Just trying to match the existing
jw_player_use_legacy
function. We've changed it.)#19 (Partially addressed in https://www.drupal.org/node/2719977. The rest of your comments, see below.)
#20 (Depends on other things, but essentially agree.)
#21 (Already resolved in https://www.drupal.org/node/2719851)
---------------------
In this section, let me mention the other issues (in descending order of importance):
Use of #player_type, default to legacy for existing sites
Related to #1, #5, #9, #12, #14. Our recommendation is to reduce from three field formatters to one. I really don't see a strong reason for three at this point. With JW Player 7, everything is contained in one array, and some minor adjustments allows us to switch from player to playlist to sources (see attached picture). It also makes the API much simpler, since a value for '#player_type' can be included in the renderable array.
Other than backward compatibility, is there a reason we need three formatters? This is a strong reason why I want old format to stay as old format for existing sites. It says in the patch: Note: if changing from Legacy to Improved, updates to templates, presets, and field configuration may be necessary.
Why should we force existing sites to use the new format, if they are happy with how it works? I prefer to put control in the hands of the developer to decide.
Per-preset cloud players
Related to #19, yes this code is missing and needs to be addressed. I think it would be best for you to see what we have in the next version of the patch, and then we can discuss. This also relates to your comment #16, which has been changed.
You said, "Instead of a single jw_player library, we will have to register one for each preset." This is exactly what we have. We are building
$libraries['jw_player_' . $preset_name]
for each saved preset, storing it in cache, and then attaching inpre_render_element
as required. It gets a bit more tricky when someone is including a#player_library_url
via API (this is missing fromhook_element_info
), but I think we have a way of handling it.Also you asked, "I'm not sure what should happen exactly when you have the license key globally and select a cloud preset?" This is not a possible situation. License key is only present during self-hosting. Cloud preset is only an option in cloud-hosting.
html_id format
Related to #3, #11, #17. First, I think we can make it less complex. The hash got introduced for extra long names. What I prefer is something like "player_34_640x480" and then hash when greater than 32 chars, or drop the hash entirely.
Regarding predictability, I don't understand how this is any more difficult than asking a developer to decipher machine names for blocks and views. Looking at id in Firebug or Chrome provides the answer. If we really want to be helpful, why not create an admin page that lists all rendered players and their matching ids?
As for hook implementations being cached, I have no problem with removing
hook_jw_player_PLAYER_ID_alter
. However we strongly needhook_jw_player_alter
, with id passed in as a variable.I'm curious, why does 8.x request users to explicitly set html_id? I understand about your previous problems of players not rendering, but this seems unnecessary (other than if a developer is building a player via API). The code should be smart enough to handle this in 95% of the cases. I don't like asking users for extra information when it's really not needed. The process should be simple.
Registering different template
Related to #9, #20, #22. Yes, JW Player (at least JW7) removes all classes. If we add a wrapper, aren't we changing the template design anyway? The goal was to create something that improves theming, but does not change the existing template. That is what we've done. I did not have the expectation that we're trying to use the existing template for the new approach.
I am confused about this comment in #20: "if we do my suggestion above, then this would change to some check to check if this is coming from the #type or not." My understanding is the pre-render works on the element, not the variables. There is no
#type
available in the template preprocess variables as far as we can see. Is there something we are missing?We added early returns in the existing template preprocess and hook functions, but as you mentioned they really don't do anything right now since we're using different templates.
Also I'm not sure what you mean about "printing video tags" in #22. JW Player renders to a div. I've never seen it done another way.
API support for old format
In #14 you said, "I think I would still prefer an approach where the primary and practically only difference between the legacy/improved setting is this. And that we support both in the API." I'm sorry, this is not something we have time to do as part of this effort. The goal of the refactoring was to create a new approach to theming, while ensuring anyone who uses the current method has no lost functionality. We have done that.
Specifying JavaScript settings
In #18 you said, "i find this a bit a weird way to specify the settings (without an explicit, unique key)". Actually I've never seen a module that implements settings in a way other than this when needing to support settings for multiple elements. Just a few modules that do it this way: Admin Menu, AdvAgg, jQuery Update, Leaflet, Hierarchical Select, Quicktabs, Views, Webform. Using the method I have here, settings for each player are loaded into a single global 'jw_player' object.
Moving code to include files
In #8 you said, "I personally don't like moving code to include files when they can't be autoloaded." I understand, but I like it because it helps with development and code manageability. Not worth discussing, we have many other things we need to get done.
Comment #28
ron_s CreditAttribution: ron_s commentedHere is a new version of the patch for review.
Comment #29
ron_s CreditAttribution: ron_s commentedThere is a picture that is mentioned in comment #27, but forgot to attach as a reference.
Comment #30
ron_s CreditAttribution: ron_s commentedIncluded is a minor update to the last patch. We did some additional testing and realized the proper libraries were not being attached when switching between Drupal presets and JW Player libraries. This version should fix the issue.
Comment #31
ron_s CreditAttribution: ron_s commentedAfter reviewing the following 7.x-1.x issue (https://www.drupal.org/node/1555960), realized that we are missing one key use case.
If a site builder selects "player" as the player type and sets the field to have multiple files, the current patch only supports the first file being rendered in a JW Player. What should be done is to loop through the files and create a new player for each individual item. This is different from a playlist, which displays multiple files in a single player.
To accomplish such a feature, it is necessary to group the JavaScript settings differently, and also change the way the players are built in JS. See attached, and also the modifications we have made to the patch in Issue #2719977. Thanks.
Comment #32
ron_s CreditAttribution: ron_s commentedJust so you're aware, we already have additional bug fixes we have made to patch #31. However I'm going to wait to post another version until we can reach consensus on the feedback outlined in comment #27. Thanks.
Comment #33
ron_s CreditAttribution: ron_s commented@Berdir, I understand 8.x has been a priority in recent months. Is there anything we can do to move 7.x-2.x toward getting the newer patches committed?
The current version of 7.x-2.x-dev is fairly useless. Probably the only version that really works is the copy we have in our development code. I'd really like to be able to allow others to benefit from the improvements we've added.
Let me know what we can do to help. Thanks.
Comment #34
aitala CreditAttribution: aitala commentedThe #31 Patch does not apply cleanly...
Comment #35
ron_s CreditAttribution: ron_s commented@aitala, as I said in my previous comment, the current -dev version is completely useless relative to these patches. It won't work without getting some of them committed first.
We invested a lot of time last year creating fixes for 7.x-2.x, and asked the module owners both publicly and privately for their help. Unless this is made a priority, there's not much we can do. Our team is not going to continue to put time toward something that is a dead end.
Comment #36
aitala CreditAttribution: aitala commentedIs there any way you can open up your repo for others to use?
I was able to hack together some of the patches to get JWP working, but I don't like doing that as I won't be maintaining the site in question and I'm not certain everything works correctly.
Thanks,
E
Comment #37
ron_s CreditAttribution: ron_s commented@aitala, I think we're just going to confuse people if there are two repositories for JW Player, and I really see no need for two separate versions of the module.
When we started working on the 7.x updates, it was with the understanding that our work would be useful in building the 8.x version. As we added more features, the same concepts could be used to incrementally improve 8.x too. Beyond the patches we've posted, there are at least five other really great ideas we have to improve the module.
However, that's not what happened. Once 8.x-1.0-beta4 was launched last August, there was no further communication from the module owners.
At this point I only see three possible options:
1) Module owners re-engage with us to get the new 7.x functionality committed.
2) Module owners give us commit authority so we can post new 7.x code.
3) MD Systems decides they no longer want to maintain the project and relinquish ownership.