User tests: Successful: Unsuccessful:
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
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Drone passes now so this is definitely the root cause (although not definitely the right fix)
@SniperSister @zero-24 can you look into drone here so that it’s passing rips when everything here is fine please?
My way to fix this was going to libraries/src/Html/HtmlHelper.php
line 778 and exchanging 'js'
with 'webcomponents/js'
I ran composer update
and npm clean-install
several times...
Ok, if we don't have a webcomponents folder anymore, as seems to be the case, then this here is wrong:
Or rather this here:
@Hackwar nope, that's correct:
HTMLHelper::_('webcomponent', 'system/joomla-toolbar-button.min.js', ['version' => 'auto', 'relative' => true]);
Break down:
I understand that, but these 2 calls reference the same file, while one exists and the other not. see dec68e1#diff-169f9020e1f708bc04e7ca0605989ac5R15
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?
Tempted for the latter to reduce b/c breaks?
Yes please! This fix wasn't without reason ;)
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)
Labels |
Added:
?
|
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
I have tested this item
I have tested this item
Buttons working again after applying PR
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-03-03 21:04:12 |
Closed_By | ⇒ | zero-24 |
Merged thanks.
Thanks!
@dgrammatiko as you posted on the original PR
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 :/