Page MenuHomePhabricator

{{PAGESINCATEGORY: Name }} does not trim its Name parameter and returns 0 unexpectedly
Open, LowestPublic

Description

Reoponing this bug that has been closed too often incorrectly without ever fixing it:

{{PAGESINCATEGORY: name |R}} does NOT (and has NEVER) worked with spaces.

This is an old bug submitted multiple times and closed each time, even if it is very easy to reproduce on ALL wikis ! My comment about this was a reminder for something that is constantly ignored, reported many times and never fixed, causing problems that are hard to find in many templates using PAGESINCATEGORY (notably in various navboxes used in Commons that are quite hard to isolate: PAGESINCATEGORY: Name is frequently used with #ifexpr:, instead of #ifexist: Name).

And it never makes senses to use the parameter of PAGEINCATEGORY without trimming it, given that there can't exist any category name (or any other page names) which is not trimmed.

And this is absolutely not related to "template arguments": the bug exists even outside any template. It's just that this bug is frequently occuring within templates that test category names with variable parameter values, where some of them may be empty (like optional prefixes/suffixes)

Note that I use the |R second parameter because this is needed in most cases in these templates to test its return value with #ifexpr, otherwise the formatted non-zero number may contain commas or other group separators causing expression errors. I don't know why PAGESINCATEGORY "formats" the returned number by default when in fact in msot cases this number will not be even displayed on the page and will only be used in expressions with {{#ifexpr: ... }} or {{#expr: ... }]}.

But the bug is the same if |R is not used: we also get 0 with an untrimmed category name.

And given that PAGEINCATEGORY is costly, it is counted: its results is cached during the parsing of the same page so that it is counted only once for the same tested category name. But:

{{PAGESINCATEGORY:Name}}, {{PAGESINCATEGORY: Name}}, {{PAGESINCATEGORY:Name }}, {{PAGESINCATEGORY: Name }} are stupidly counted as 4 separate invokations as if the tested name was different (apparently 4 distinct costly SQL queries are performed, only one is useful and gets the expected result). If the name parameter was trimmed each time, this would count for only 1 invokation and it would be less costly.

So please trim the category name, exactly like what is done:

  • for creating wikilinks to categories with [[:Category: Name]] or [[:Category: Name|display]], or
  • for categorizing a page with [[Category: Name]] or [[Category: Name |sortkey]] or
  • for testing it with {{#ifexist: Category: Name | ...}}.

We still need the workaround {{PAGESINCATEGORY:{{#titleparts: name }}|R}}.

Event Timeline

Verdy_p updated the task description. (Show Details)
Verdy_p updated the task description. (Show Details)
Verdy_p updated the task description. (Show Details)
Verdy_p updated the task description. (Show Details)

Reoponing this bug that has been closed too often incorrectly without ever fixing it:

@Verdy_p: You did not "reopen this bug" (if you meant T16237), you created a new ticket.
@Verdy_p: Please do read https://www.mediawiki.org/wiki/How_to_report_a_bug instead of continuing to ignore it. You have been asked many times to do so.

@Verdy_p: Please read, understand, and follow https://www.mediawiki.org/wiki/How_to_report_a_bug .
As written in T16237, "PAGESINCATEGORY works with spaces, there is possible another issue when using template arguments".

It has been reported multiple times as part of relevant bugs that were partly solved and closed, even when discussing the proposd solutions).

Each time it has been ignored, as if it was not a priority, and various users repeatedly condue posting about PAGESINCATEGORYnot working, and you close them, most of the times this was because their tests included untrimmed spaces !
This is a frequent bug !

As as I wrote, your comment repeating the statement "there is possible another issue when using template argument" is simply completely false (this is just your untested assumption).
This has absolutely NOTHING to do with template parameters ands can be reproduced outside of any template call.
This is not caused by template expansion, which works normally and expands with spaces as expected. But this bug is only about how PAGESINCATEGORY treats its effective parameter.

The examplar case is with this type of code (found in many templates), not working as expected:

{{#ifexpr: {{PAGESINCATEGORY:{{{prefix|}}} Portugal {{{suffix|}}}|R}}
| [[Category:{{{prefix|}}} Portugal {{{suffix|}}}]]<!--
  -->{{#ifexpr: {{PAGESINCATEGORY:{{{prefix|}}} Portugal {{{suffix|}}}|R}} > 200
  | [[Category:Overpopulated category|{{{prefix|}}} Portugal {{{suffix|}}}]]
  }}
}}

Your suggestion for testing each parameter just createds tricky code. My workaround (using #titlepart:) makes this much simpler: we just need to trim the computed value.

This code works (but displays categories when they exist even if they are empty) but partly for the second case (when prefix or suffix are empty, the second test always fails, and if it worked, the computed key would contain a leading space, generally not wanted to correctly sort members in "Category:Overpopulated category"):

{{#ifexist: Category:{{{prefix|}}} Portugal {{{suffix|}}}
| [[Category:{{{prefix|}}} Portugal {{{suffix|}}}]]<!--
  -->{{#ifexpr: {{PAGESINCATEGORY:{{{prefix|}}} Portugal {{{suffix|}}}|R}} > 200
  | [[Category:Overpopulated category|{{{prefix|}}} Portugal {{{suffix|}}}]]
  }}
}}

And the workaround indicated above applies:

{{#ifexpr: {{PAGESINCATEGORY:{{#titleparts: {{{prefix|}}} Portugal {{{suffix|}}}|R}}}}
| [[Category:{{{prefix|}}} Portugal {{{suffix|}}}]]<!--
  -->{{#ifexpr: {{PAGESINCATEGORY:{{#titleparts: {{{prefix|}}} Portugal {{{suffix|}}} }}|R}}}} > 200
  | [[Category:Overpopulated category|{{#titleparts: {{{prefix|}}} Portugal {{{suffix|}}} }}]]
  }}
}}

But this workaround should not be needed (this is not what template authors expect, the trimming is assumed to work like with #ifexist:). You must not insert any other spaces, as the following breaks the workaround, by reinserting surrounding spaces around the result of #titleparts:, and once again these two spaces are making PAGESINCATEGORY: not working again:

{{#ifexpr: {{PAGESINCATEGORY: {{#titleparts: {{{prefix|}}} Portugal {{{suffix|}}} |R}} }}
| [[Category:{{{prefix|}}} Portugal {{{suffix|}}}]]<!--
  -->{{#ifexpr: {{PAGESINCATEGORY: {{#titleparts: {{{prefix|}}} Portugal {{{suffix|}}} }}|R}} }} > 200
  | [[Category:Overpopulated category|{{#titleparts: {{{prefix|}}} Portugal {{{suffix|}}} }}]]
  }}
}}

If this bug is solved (i.e. PAGESINCATEGORY: correctly trims its Name parameter), the code could be reduced/simplified to:

{{#ifexpr: {{PAGESINCATEGORY: {{{prefix|}}} Portugal {{{suffix|}}} |R}}
| [[Category:{{{prefix|}}} Portugal {{{suffix|}}}]]<!--
  -->{{#ifexpr: {{PAGESINCATEGORY: {{{prefix|}}} Portugal {{{suffix|}}} }}|R}} > 200
  | [[Category:Overpopulated category|{{#titleparts: {{{prefix|}}} Portugal {{{suffix|}}} }}]]
  }}
}}

(the last call of #titleparts: just above is still needed for correctly setting the trimmed sort key, but this is not part of this bug; but if this last call is removed, the tested category name will still be categorized as expected, but possibly sorted incorrectly for some expectations).

Not trimming the parameter just forces authors to use tricky code, more complicate than needed, and even if we use the workaround, some other editors will want to insert extra spaces to separate groups of braces, breaking the workaround, or will even want to remove the use of #titleparts: as if it was not needed.

This is an unstable situation (this has already happened where we needed to revert the insertion of these extra spaces in the parameter of PAGESINCATEGORY: very few people are aware of this problem, and even you are not aware of this problem, as proven by your false statement).

The most basic test you can test any wiki page (including your own user talk page), outside of any template is to just use:

[[:Category:Categories]] contains {{PAGESINCATEGORY: Categories }} pages.

And it will contantly display "0 pages" (clearly unexpected false result). This is seen at least in Wikimedia Commons.

@Verdy_p: Please read, understand, and follow https://www.mediawiki.org/wiki/How_to_report_a_bug .
As written in T16237, "PAGESINCATEGORY works with spaces, there is possible another issue when using template arguments".

That comment is from me and I have tested it locally and now on test.wikipedia.org - Please see https://test.wikipedia.org/w/index.php?title=T257535&oldid=441519 for some examples with spacing and the limit report shows only 1 usage ("Expensive parser function count: 1/500"). That means for me: It works (with the current mediawiki version as of written it is 1.35alpha)

test.wikipedia is not a production server, so may be there was a fix on the latest release. This still does not work on Commons where the workaround was and is still needed. It's possible that the trimming is partial (drops only one space, not tabs/newlines possibly found in some (positonal) template parameters, but my test cases was for calls of templates written as a single line without any space given, the only space were as is the exemplar code above, in navboxes for geographic entities).
So for Commons this is still not solved.

test.wikipedia is at the moment on the same version as commons: 1.35.0-wmf.40 (rMWbca133d4715f) 11:36, 9 July 2020

You can copy over the wikitext from test.wikipedia to the sandbox on commons and using the preview to see that it works.
https://commons.wikimedia.org/wiki/Commons:Sandbox

Please give a broken example at one of the wmf wikis which demostrate the problem.
Without a template name or a page name using a specific template (please give the name) it is not possible to follow this task.

Please also try to see the page logged out, maybe there is a user preference involved like VisualEditor.

Maybe the problem is by using parsoid or lua, but the php parser does not have the problem with spaces.