Page MenuHomePhabricator

Linter false positive: image caption ending in "px" is incorrectly detected as a Linter error
Closed, ResolvedPublicBUG REPORT

Description

Steps to Reproduce: See https://en.wikipedia.org/w/index.php?title=User:Jonesey95/sandbox&oldid=1010663816 which generates a Linter bogus image options error.

Actual Results:
Linter reports a bogus image options error when a specified px value is present and the caption ends in "px".

Expected Results:
Linter should not report an error.

This is yet another side effect of the failure of the Image parser to include an option to specify "caption=" to force a particular option to be the caption for an image.

This false positive can be worked around by adding an nbsp or any other text after the end of the caption.

This Linter false positive is new. It is probably related to the recent improvement deployed as a fix for T215999.

Event Timeline

This item could have been titled:
Linter false positive: "Foo px" as caption is incorrectly detected as a Linter error
It happens whether or not digits precede px.

Jonesey95 renamed this task from Linter false positive: "Foo 100px" as caption is incorrectly detected as a Linter error to Linter false positive: image caption ending in "px" is incorrectly detected as a Linter error.Mar 10 2021, 1:34 AM
Arlolra subscribed.

Yeah, this is fallout from T215999

The alias for width is,

{
  "name": "img_width",
  "aliases": [
    "$1px"
  ],
  "case-sensitive": ""
},

If it fails further validation, it's marked as bogus. The legacy parser, however, will take any failing option and consider it the caption.

A note from someone who regularly fixes lint errors: I've found this linter behaviour useful to detect people using the letter O instead of the digit zero, as in "2OOpx". On nlwiki there have been no other true positives (with suffix "px"). I suggest to adapt this to only consider typos like ppx and characters that resemble digits as bogus. Such as Latin O or o, but perhaps also Cyrillic З and others.

A note from someone who regularly fixes lint errors: I've found this linter behaviour useful to detect people using the letter O instead of the digit zero, as in "2OOpx". On nlwiki there have been no other true positives (with suffix "px"). I suggest to adapt this to only consider typos like ppx and characters that resemble digits as bogus. Such as Latin O or o, but perhaps also Cyrillic З and others.

I see how this bug could be useful, but it is still a bug that did not exist until T215999 was implemented. Ending a caption with "px" is valid and should not generate a Linter error.

If you want to detect errors in the rare instance where an erroneous size is provided with no caption, a database query should be able to find that condition for you. You might also be able to get it added to the long list of reports generated by WPCleaner.

Change 861957 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/services/parsoid@master] Allow captions ending in px

https://gerrit.wikimedia.org/r/861957

Arlolra triaged this task as Medium priority.
Arlolra moved this task from Backlog to Code Review on the Content-Transform-Team-WIP board.

Change 861957 merged by jenkins-bot:

[mediawiki/services/parsoid@master] Allow captions ending in px

https://gerrit.wikimedia.org/r/861957

Change 864839 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/vendor@master] Bump parsoid to 0.17.0-a8

https://gerrit.wikimedia.org/r/864839

Change 864839 merged by jenkins-bot:

[mediawiki/vendor@master] Bump parsoid to 0.17.0-a8

https://gerrit.wikimedia.org/r/864839

On nlwiki, Special:LintErrors still has a false positive of this type present (here it lists Sepx). I've edited the page (without fixing the lint error) but the page remains in the list. How to clear it?

On nlwiki, Special:LintErrors still has a false positive of this type present (here it lists Sepx). I've edited the page (without fixing the lint error) but the page remains in the list. How to clear it?

I tried parsing that page locally before and after the fix merged here and it seems to work as expected. This issue is likely that the lints weren't cleared, which is an ongoing issue tracked in T246403#8455311