? Pending

User tests: Successful: Unsuccessful:

avatar infograf768
infograf768
15 Oct 2017

Summary of Changes

Check if com_fields is enabled

Testing Instructions

Disable the com_fields component via Extensions=> Manage=>Manage
Edit an article and click on the Field icon in the editor toolbar.
=> 404

Patch and test again: the button will not display

avatar joomla-cms-bot joomla-cms-bot - change - 15 Oct 2017
Category Front End Plugins
avatar infograf768 infograf768 - open - 15 Oct 2017
avatar infograf768 infograf768 - change - 15 Oct 2017
Status New Pending
avatar Quy Quy - test_item - 15 Oct 2017 - Tested successfully
avatar Quy
Quy - comment - 15 Oct 2017

I have tested this item successfully on e9fdd4d


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

avatar laoneo laoneo - test_item - 16 Oct 2017 - Tested successfully
avatar laoneo
laoneo - comment - 16 Oct 2017

I have tested this item successfully on e9fdd4d


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

avatar franz-wohlkoenig franz-wohlkoenig - change - 16 Oct 2017
Status Pending Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 16 Oct 2017

RTC after two successful tests.

avatar dgt41
dgt41 - comment - 16 Oct 2017

Thanks @infograf768 for patching this. One note here tho,
I would like us to change the API of the buttons by adding 2 more vars in the object button:
realName string, which would be the real name of the button (eg pig_xtd_articles) now the name is some class for the icomoon icon
component string or null which would refer to the component associated with the button

Then adding just one check, similar to what you've done here, in Editor getButtons() we have this functionality for every component, without hardcoding anything and without even loading the button if the component is disabled.

I guess what I'm proposing should go to J4, but even J3 should be happy as we're adding functionality without breaking any existing. @mbabker @wilsonge what do you think?

avatar infograf768
infograf768 - comment - 16 Oct 2017

Looks like a nice idea.

avatar wilsonge
wilsonge - comment - 19 Oct 2017

Not all xtd buttons have to go into a component I guess? And also the other way you could definitely be dependent on multiple components... The plugin should bail early on it's component deps rather than us trying to force something through the system.

avatar infograf768
infograf768 - comment - 19 Oct 2017

we could maybe merge this one until another possible solution is decided.

avatar dgt41
dgt41 - comment - 19 Oct 2017

Not all xtd buttons have to go into a component I guess

Sure, thats the null || string option in the variable, if null no component dependency, the code curries on normally..
The difference between checking inside the plugin or in the editor is mainly for some possible performance gain (the system will not load/execute any xtd's when the component that they're depending is disabled). Anyways the gain should be marginal (unless someone has 1000 xtd buttons)...

avatar wilsonge wilsonge - change - 19 Oct 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-10-19 15:09:47
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 19 Oct 2017
avatar wilsonge wilsonge - merge - 19 Oct 2017

Add a Comment

Login with GitHub to post a comment