Please alpha order the new language strings.

Imho, the real issue is coming from the module inconsistently calling the plugin event with a context of com_content.article instead of the 'mod_articles_news.content' it uses in a earlier call.
If it always would call with the proper context, then the plugins itself could decide if they should show or nor in that context.

I totally agree Bakual sorry I missed that. I will update the code accordingly and then as you said a plugin can check the context and decided itself.

(restarted travis)

Do you need me to do anything to help this along?

I think the travis failure is unrelated to this PR - maybe @wilsonge can check

Yup it's unrelated. For tests to pass in this PR we need to have the PR rebased onto staging so that it contains this commit:
41807b2 (which fixes this issue)

@coalaweb please can you do as George says and rebase your PR

On 10 March 2016 at 16:21, George Wilson notifications@github.com wrote:

Yup it's unrelated. For tests to pass in this PR we need to have the PR
rebased onto staging so that it contains this commit:
41807b2
41807b2
(which fixes

Which fixes this issue

—
Reply to this email directly or view it on GitHub
#8610 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

Will do I just need a minute.

I just rebased

Am I missing something?

Yes the Joomla Code style checks are failing :) Errors that need fixing are listed here https://travis-ci.org/joomla/joomla-cms/jobs/115116864#L1361

OK cool I'll take a look.

Thanks for the help, its all good now ;)

No thank you for sticking with it :)

Just a question is this not RTC because of the lack of testing?

Thats correct - it needs at least two succesfull user test

Cool I will throw together a plugin to make it easier for people to test and I will hit the old social media to see it I can get a couple of people testing.

To help get my PR tested and included I have made it even easier to test just follow these steps.

1. First create a new article or use an existing one with a bit of text.

2. Next create an Articles - Newsflash module.

3. Lastly install and publish the plugin Content - CW Mod News Test and you should now see the context of the article is com_content.article and the module is mod_articles_news.content So I think it stands to reason that my PR that the changes:

From
$results = $app->triggerEvent('onContentAfterDisplay', array('com_content.article', &$item, &$params, 1));
$results = $app->triggerEvent('onContentBeforeDisplay', array('com_content.article', &$item, &$params, 1));

To
$results = $app->triggerEvent('onContentAfterDisplay', array('mod_articles_news.content', &$item, &$params, 1));
$results = $app->triggerEvent('onContentBeforeDisplay', array('mod_articles_news.content', &$item, &$params, 1));

Would be a worth while change or am I missing something?

Cheers,

Steve

I have not tested this item.

I have followed the steps for testing and it says com_content.article for the context of the article, and says mod_articles_news.content on the module. But applying patch on Patch Tester for this issue doesn't change anything. So I'm not sure if its successful or not.

I have tested this item ? unsuccessfully on b7c14bd

I think has been some confusion the patch won't change anything visual I have included the plugin to show that an article is using com_content.article while the module is using mod_articles_news.content as you saw with the plugin installed. Based on that it stands to reason that the module's plugin trigger events should be on the module content mod_articles_news.content which is what the patch changes.

You might want to update you failed test.

Cheers,

Steve

This PR asks for:

mod_articles_news.content

PR suggests: #11339 , to use context

mod_articles_news.article

Now we have: (in existing module code)

com_content.article

I would like: (something to clearly give out both the used content type and the location of usage) e.g.

com_content.article@mod_articles_news

Please note, i am just summarising, i am not asking for any changes, or anything

• in fact i don't know if some intended B/C breaks , can be published e.g. a few months time prior to e.g. official release of J3.7, then most developers can update their extensions, before they happen

so that we do not have to wait for J4 for some improvements, like this (= knowing not only what is displayed, but also where it is displayed)

• my interest is not in changing or not changing the module, but it is in having a proper format for "context" that will be used consistently everywhere

I have tested this item ? unsuccessfully on b7c14bd

Either the test is incomplete or there is no difference.
The Locations of Article & Module remain the Same

@icampus Pizza, Bugs&Fun

I have tested this item ✅ successfully on b7c14bd

Context have been changed, checked manually with debugger.

Curiensol please see my earlier comment. Thanks.

I have tested this item ✅ successfully on b7c14bd

Sorry about before, I have tested again and it works as it is said in the suggested steps.

I have tested this item ✅ successfully on b7c14bd

I tested this @icampus PBF and confirm that this works. I followed the suggested steps and fix works as suggested.

Yes it "works", that is not what is most important here,

questions are

• what is a proper context to be used ? (please see my previous comment)
• there is a big B/C break, for e.g. 2%-5% ?? (i do not know percentage) sites using the module and along with some plugins (please see my previous comment)

Plugin replacements will stop working, since all 3rd plugins will not to be updated in "time"

be honest i do not care much about this module, as much i mind the "proper context" question

(users will care), also site managers will mind when they hear the users complaints, plus they will be annoyed by delay in plugin updating, (and not every plugin will be updated either)

@ggppdk You raise a valid question as to what is the correct context. Looking at what is in core now the structure allows follows the name of the view. com_content.archive for the archive view, com_tags.tag for the tag view. So for this PR I would say we need com_content.mod_articles_news. Another variation on what you proposed. To be B/C we would need to call both and deprecate the old one.

As there are conflicts here, I've created an updated PR with #12289