? Pending

User tests: Successful: Unsuccessful:

avatar wilsonge
wilsonge
3 Mar 2019

A b/c break in #19972 has caused web components scripts to no longer be enqueued for use by core.js - this reworks the webcomponent jhtml call to be a bit cleaner and work around the b/c break

avatar wilsonge wilsonge - open - 3 Mar 2019
avatar wilsonge wilsonge - change - 3 Mar 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 3 Mar 2019
Category Libraries
avatar wilsonge
wilsonge - comment - 3 Mar 2019

@dgrammatiko as you posted on the original PR

Edge case if someone used this code to actually replace the values in the storage but then again I don't think that many people are actually using or even understand the Joomla.storageOptions.

I think that’s exactly what we are doing with webcomponents. We have added a single element array and rely on it merging into the existing array elements. After that PR the last entry overwrites the one before it :/

avatar wilsonge
wilsonge - comment - 3 Mar 2019

Drone passes now so this is definitely the root cause (although not definitely the right fix)

avatar wilsonge
wilsonge - comment - 3 Mar 2019

@SniperSister @zero-24 can you look into drone here so that it’s passing rips when everything here is fine please?

avatar dgrammatiko
dgrammatiko - comment - 3 Mar 2019

@wilsonge changing it to array_merge_recursive fixes this?

avatar Hackwar
Hackwar - comment - 3 Mar 2019

My way to fix this was going to libraries/src/Html/HtmlHelper.php line 778 and exchanging 'js' with 'webcomponents/js'

avatar dgrammatiko
dgrammatiko - comment - 3 Mar 2019

@Hackwar I don't think there is a webcomponents folder anymore...

avatar Hackwar
Hackwar - comment - 3 Mar 2019

I ran composer update and npm clean-install several times...

avatar Hackwar
Hackwar - comment - 3 Mar 2019

Ok, if we don't have a webcomponents folder anymore, as seems to be the case, then this here is wrong:

HTMLHelper::_('webcomponent', 'system/joomla-toolbar-button.min.js', ['version' => 'auto', 'relative' => true]);

avatar Hackwar
Hackwar - comment - 3 Mar 2019

Or rather this here:

HTMLHelper::_('webcomponent', 'system/fields/joomla-toolbar-button.min.js', ['relative' => true, 'version' => 'auto', 'detectDebug' => true]);

avatar dgrammatiko
dgrammatiko - comment - 3 Mar 2019

@Hackwar nope, that's correct:
HTMLHelper::_('webcomponent', 'system/joomla-toolbar-button.min.js', ['version' => 'auto', 'relative' => true]);

Break down:

  • 'webcomponent' calls the fn webcomponent
  • 'system/joomla-toolbar-button.min.js' The path
  • 'relative' => true] The path is relative eg real path: system/js/joomla-toolbar-button.min.js
  • 'version' => 'auto' Version string applied, eg: ?fdkjhkldjfhk
avatar Hackwar
Hackwar - comment - 3 Mar 2019

I understand that, but these 2 calls reference the same file, while one exists and the other not. see dec68e1#diff-169f9020e1f708bc04e7ca0605989ac5R15

avatar dgrammatiko
dgrammatiko - comment - 3 Mar 2019

@Hackwar yeah that's a bug, you can blame me for that. In sort the toolbar is not a field thus the script shouldn't live in that folder

avatar wilsonge
wilsonge - comment - 3 Mar 2019

@Hackwar I fixed that one already #24074

avatar wilsonge
wilsonge - comment - 3 Mar 2019

The problem is, is where and how we want to fix the “break” given this was merged and released in August in 3.8. We can change the function here back/to array recursive merge OR we can change web components understanding the value will get overwritten. Tempted for the latter to reduce b/c breaks?

avatar bembelimen
bembelimen - comment - 3 Mar 2019

Tempted for the latter to reduce b/c breaks?

Yes please! This fix wasn't without reason ;)

avatar roland-d
roland-d - comment - 3 Mar 2019

@wilsonge Tested this successfully. Current 4.0-dev the toolbar buttons are broken. This fix makes it work again.

avatar dgrammatiko
dgrammatiko - comment - 3 Mar 2019

This fix wasn't without reason

If the only use case is tinyMCE then probably that's not a big problem as I see J4 coming out with the new tinyMCE which of course will break B/C with the old crappy way of initialising the editor (in sort will not use scriptOptions)

avatar wilsonge wilsonge - change - 3 Mar 2019
Labels Added: ?
avatar wilsonge
wilsonge - comment - 3 Mar 2019

Reworked the webcomponent part and cleaned it up at the same time. Please test this now. In retrospect I really wish we'd done a recursive merge instead of recursive replace. But unfortunately the ship has sailed - and given it's easy to work around this for webcomponents here - I'm not going to try and introduce an unncessary b/c break again

avatar wilsonge wilsonge - change - 3 Mar 2019
The description was changed
avatar wilsonge wilsonge - edited - 3 Mar 2019
avatar Hackwar Hackwar - test_item - 3 Mar 2019 - Tested successfully
avatar Hackwar
Hackwar - comment - 3 Mar 2019

I have tested this item successfully on db78ac2


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24075.

avatar roland-d roland-d - test_item - 3 Mar 2019 - Tested successfully
avatar roland-d
roland-d - comment - 3 Mar 2019

I have tested this item successfully on db78ac2

Buttons working again after applying PR


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24075.

avatar zero-24 zero-24 - change - 3 Mar 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-03-03 21:04:12
Closed_By zero-24
avatar zero-24 zero-24 - close - 3 Mar 2019
avatar zero-24 zero-24 - merge - 3 Mar 2019
avatar zero-24
zero-24 - comment - 3 Mar 2019

Merged thanks.

avatar wilsonge
wilsonge - comment - 3 Mar 2019

Thanks!

Add a Comment

Login with GitHub to post a comment