Page MenuHomePhabricator

Phase out legacy error, warning and success classes for usage outside the parser
Closed, ResolvedPublic

Description

These were identified to be one of the few remaining legacy classes.

They are extensively used within MediaWiki and extensions, making deprecation or removal more intensive than other styles in legacy.less.

These currently appear unstyled in the mobile site and will soon appear unstyled in the desktop site.

Consumers relying on styling from MediaWiki should be updated to either

  1. Provide their own styles for these classes via MediaWiki:Common.css/template style rules
.error,
.warning,
.success {
        font-size: larger;
}

.error {
        color: #d33;
}

.warning {
        color: #ac6600;
}

.success {
        color: #14866d;
}
  1. Using the .messagebox,.errorbox,.warningbox,.successbox classes

Identified usages

.error

Usages of .error: https://codesearch.wmcloud.org/search/?q=class%3D%22(%3F%3A%5B%5E%22%5Cs%3E%5D*%5Cs%2B)*%3Ferror%5B%5Cs%22%5D&i=nope&files=&excludeFiles=&repos=

  • Parser/ ParserFunctions (Also Parsoid?) - export <strong class="error"></strong> for parsing errors
    • These aren't going away, they'll be moved instead (T281228), but this shouldn't have any user-facing impact.
  • EditPage::handleStatus - <div class="error"></div> for hooks.

.success

Usages of .success: https://codesearch.wmcloud.org/search/?q=class%3D%22(%3F%3A%5B%5E%22%5Cs%3E%5D*%5Cs%2B)*%3Fsuccess%5B%5Cs%22%5D&i=nope&files=&excludeFiles=&repos=

.warning

Usages of .warning: https://codesearch.wmcloud.org/search/?q=class%3D%22(%3F%3A%5B%5E%22%5Cs%3E%5D*%5Cs%2B)*%3Fwarning%5B%5Cs%22%5D&i=nope&files=&excludeFiles=&repos=

This one is rare.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

error at least is used onwiki. Please do a wider search for the above. Representative search for en.wp. success looks much less used; almost every hit is a false positive. Overall, warning has the fewest hits. All of that however is just one wiki.

@Jdlrobson You marked this for inclusion in Tech News. Not sure I get the context here, so feel free to correct me, but something like this?

The [[:w:en:CSS|CSS]] classes <code>.error</code>, <code>.warning</code> and <code>.success</code> do not work for mobile readers if they have not been specifically defined on your wiki. They will soon not work on desktop. They can be defined in [[MediaWiki:Common.css/template]] instead. You can also use <code>.messagebox,</code> <code>.errorbox,</code> <code>.warningbox</code> or <code>.successbox</code> instead.

Who should act on this?

Who should act on this?

Gadget developers, template editors should review any code which depends on these classes and flag any potential problems. At time of writing we are aiming to remove support for these in all skins by June 1st.

Tacsipacsi added a subscriber: Tacsipacsi.

ParserFunctions not only emits, but also consumes .error: rEPFN includes/ParserFunctions.php:169 (at b31c726461de) finds other ParserFunctions functions returning errors, but not only those, anything else that contains strong.error, span.error, p.error, div.error, like another extension’s/core’s error message, or even a template that uses the same error report mechanism. It’s also true the other way round, e.g. Lua modules may count on PF functions returning an element with class="error" on error. Even changing other extensions/core may break things, so I’m not sure how (or even whether) to proceed here. (Dropping .warning and .success is harmless in this regard.)

By the way, the literal search values were way too strict, I’ve updated them with regexes inspired by the above PF regex. There are no magnitude changes, but it finds for example more than twice as many .error usages as before.

Given how PF functions (I assume that's #iferror without looking at the context) I think that's definitely in the realm of "must keep" error. That might not preclude removing the CSS, but I think it would be an error [heh jokes] if that was removed even after an indication of such general use.

The [[:w:en:CSS|CSS]] classes <code>.error</code>, <code>.warning</code> and <code>.success</code> do not work for mobile readers if they have not been specifically defined on your wiki. They will soon not work on desktop. They can be defined in [[MediaWiki:Common.css/template]] instead. You can also use <code>.messagebox,</code> <code>.errorbox,</code> <code>.warningbox</code> or <code>.successbox</code> instead.

Why is [[MediaWiki:Common.css/template]] suggested? Does it automatically load by system?

Because I misread the comment! It's been fixed.

Given the prevalence, perhaps we're better off moving these styles to the feature content-parser-output for 1.37 to give more time to transfer.

ParserFunctions not only emits, but also consumes .error: rEPFN includes/ParserFunctions.php:169 (at b31c726461de) finds other ParserFunctions functions returning errors, but not only those, anything else that contains strong.error, span.error, p.error, div.error, like another extension’s/core’s error message, or even a template that uses the same error report mechanism. It’s also true the other way round, e.g. Lua modules may count on PF functions returning an element with class="error" on error. Even changing other extensions/core may break things, so I’m not sure how (or even whether) to proceed here. (Dropping .warning and .success is harmless in this regard.)

I think it would help a lot if things that do not need to use class="error" (e.g. AbuseFilter/CentralAuth special pages) can be moved to class="errorbox". Parser output related styles should be handled with more care.

By the way, the literal search values were way too strict, I’ve updated them with regexes inspired by the above PF regex. There are no magnitude changes, but it finds for example more than twice as many .error usages as before.

Thank you. I hadn't managed to get that right, and just used the one that gave me the least false positives.

error at least is used onwiki. Please do a wider search for the above. Representative search for en.wp. success looks much less used; almost every hit is a false positive. Overall, warning has the fewest hits. All of that however is just one wiki.

I wanted to use https://global-search.toolforge.org/, but the amount of false positives is staggering to the point of uselessness. Once I find out how to make it behave, I'll add the results (but I should've mentioned that in the task description).

I think it would help a lot if things that do not need to use class="error" (e.g. AbuseFilter/CentralAuth special pages) can be moved to class="errorbox". Parser output related styles should be handled with more care.

While this should be done where possible, some contexts, like inline errors from various extensions (ParserFunctions named above, also can name Scribunto and Cite) will always require using .error or some other name for it (since .errorbox will not render correctly in a paragraph), so I don’t think removing it is wise. .success and .warning are remnants of (I think?) Special:Preferences not using .warningbox/.successbox before, so they can be removed, but I would like to join voices saying that .error should be kept.

I mean, .error is one line of CSS, I don't think adding it to content-parser-output should hurt. Keeping it wouldn't be a maintenance burden then. Scope it to .mw-parser-output, if we must - that doesn't affect extensions.

My first move would be to change things that can safely use .errorbox and friends.

@Jdlrobson Will Minerva be using content-parser-output? If so, we could just move .error there (font-size: larger optional) and phase out everything else.

Let's track this in a new task. We could use content-parser-output provided we scope the rules. Yes Minerva will be using it too.

Is there anyway to replicate this locally so I can see how they look?

Change 682715 had a related patch set uploaded (by Yaron Koren; author: Yaron Koren):

[mediawiki/extensions/PageForms@master] Replace deprecated "error", "warning" CSS classes

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

Change 682715 merged by jenkins-bot:

[mediawiki/extensions/PageForms@master] Replace deprecated "error", "warning" CSS classes

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

Let's track this in a new task. We could use content-parser-output provided we scope the rules. Yes Minerva will be using it too.

I've created T281228: Provide styling for parser-outputted .error class in content-parser-output for that and I will re-scope this task to focus on the non-parser parts.

Is there anyway to replicate this locally so I can see how they look?

If you have ParserFunctions installed, just use {{#expr:garbage}}. I haven't found anything that works without it yet. The parser tests suggest that it should be possible, so I'll look in that next.

Mainframe98 renamed this task from Phase out legacy error, warning and success classes to Phase out legacy error, warning and success classes for usage outside the parser.Apr 27 2021, 9:19 AM
Mainframe98 updated the task description. (Show Details)

Change 682915 had a related patch set uploaded (by Mainframe98; author: Mainframe98):

[mediawiki/core@master] Don't use classes error, success and warning outside parser

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

Change 682960 had a related patch set uploaded (by Mainframe98; author: Mainframe98):

[mediawiki/extensions/AbuseFilter@master] Don't use p class="success" for success messages

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

Er, should these even be used inside the parser? Like what are they even for that a successbox wouldn't cover? Why have this variation?

Change 682973 had a related patch set uploaded (by Mainframe98; author: Mainframe98):

[mediawiki/extensions/CentralAuth@master] Don't use .success and .error for success and error messages

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

Er, should these even be used //inside// the parser? Like what are they even for that a successbox wouldn't cover? Why have this variation?

It's a good question and it depends on who the errors are for. If the errors are meant for readers, then definitely should use errorBox. If they should be considered internal users, and not be shown to readers then they probably need different handling and dedicated styles in the parser.

Er, should these even be used //inside// the parser? Like what are they even for that a successbox wouldn't cover? Why have this variation?

It's a good question and it depends on who the errors are for. If the errors are meant for readers, then definitely should use errorBox. If they should be considered internal users, and not be shown to readers then they probably need different handling and dedicated styles in the parser.

For .error, the parser outputs <p> (and sometimes (through extensions) <strong>) inline, wherever they appear. Using .errorbox (optionally on a <div>) might break the flow.

.warning and .success aren't used in the parser.

Change 682915 merged by jenkins-bot:

[mediawiki/core@master] Don't use classes error, success and warning outside parser

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

Hey @ssastry we are auditing skin styles, and looking to deprecate/repurpose use of the error class and consistently style them across skins. Could you provide some guidance on parser errors (or delegate to somebody that can)? Are these meant for end users?

Currently if I create a parser error I see this in Minerva:

Screen Shot 2021-04-27 at 3.32.53 PM.png (752×2 px, 114 KB)

and this in Vector:

Screen Shot 2021-04-27 at 3.33.33 PM.png (1×2 px, 331 KB)

Who are these for? Readers? Editors? Developers?

If users, could we make use of the errorbox class and improve the error message to make it more actionable e.g. "There was an error parsing this page. The error is Expression error: Unrecognized word "garbage"." ?

Screen Shot 2021-04-27 at 3.34.02 PM.png (630×2 px, 117 KB)

If developers, perhaps these should he hidden by default rather than confuse users?

Good questions.

Hey @ssastry we are auditing skin styles, and looking to deprecate/repurpose use of the error class and consistently style them across skins. Could you provide some guidance on parser errors (or delegate to somebody that can)? Are these meant for end users?
...

Who are these for? Readers? Editors? Developers?

If users, could we make use of the errorbox class and improve the error message to make it more actionable e.g. "There was an error parsing this page. The error is Expression error: Unrecognized word "garbage"." ?
...

If developers, perhaps these should he hidden by default rather than confuse users?

We should chat about it. ( cc @cscott @Arlolra @Sbailey). I am not familiar with what guidance exists for this. But, certain kinds of errors from templates and extensions are only shown on preview (so, editors usually). Although, in some cases, I've noticed bright-red Cite errors are displayed during read views as well -- I suppose the incentive is to get someone to notice and fix them? As for other wikitext markup, there is no such thing as "markup error" right now since the wikitext -> HTML transform generates *some* output. But, we have various lint errors that Parsoid flags for editors to fix up out-of-band.

So, overall, this is probably more of a product / UX level of discussion that need to be had as to how warnings / lints / errors need to be handled in various components (markup, templates, extensions, installed gadgets, JS modules).

At least on the Parsoid output end, certain kinds of errors are flagged in the HTML via https://www.mediawiki.org/wiki/Specs/HTML/2.2.0#Error_handling and we are beginning to consolidate behind that mechanism so different clients can take appropriate actions suitable to their context (reading and editing clients might do different things). We could potentially start stuffing linter errors within some such framework as well.

But, I know I didn't answer the very specific question you asked. I don't think that your typical reader might be able to fix these errors, so that argues for suppressing them or displaying them in differently than they are now. But, that said, there is a case to be made about making broken stuff obvious on a page so someone sees it and fixes it. We will chat about this in one of our meetings, but we welcome any guidance / recommendations the web team might have based on end-user UX considerations. But, whatever the decision, I think it is useful to think about it all coherently across all content producing components (top-levle page markup, templates, extensions, gadgets, JS modules, etc.) so there is more coherent handling for them.

(This was excluded from last Tech News, since it didn't seem to be ready to be announced. Please let me know when it is.)

@Johan this can be announced. The main questions are around the future of this styling for parser errors. That doesnt impact editors making their own decisions about how to handle their scripts/gadget styling.

.error remains available for template authors to mark output as an error report, so this affects ONLY gadget/script authors from now on, NOT template authors, right?

.error remains available for template authors to mark output as an error report, so this affects ONLY gadget/script authors from now on, NOT template authors, right?

Correct. Provided the content is outputted by the parser.

CentralAuth and PageForms call $this->getOutput()->wrapWikiMsg that method is a wrapper for OutputPage::addParserOutputText
With the changes proposed in T279388, we'll change mw-parser-output to mw-body-content and these should work as is. @Mainframe98 does that sound right to you?

CentralAuth and PageForms call $this->getOutput()->wrapWikiMsg that method is a wrapper for OutputPage::addParserOutputText
With the changes proposed in T279388, we'll change mw-parser-output to mw-body-content and these should work as is. @Mainframe98 does that sound right to you?

Only for cases where they use .error - there's some .success usages too. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralAuth/+/682973 just converts them to .errorbox/.successbox to be consistent.

Ah yes. Thanks for the sanity check !

If users, could we make use of the errorbox class and improve the error message to make it more actionable e.g. "There was an error parsing this page. The error is Expression error: Unrecognized word "garbage"." ?

Screen Shot 2021-04-27 at 3.34.02 PM.png (630×2 px, 117 KB)

error can and will appear inline and is for readers/editors to correct usually (Cite is the most usual case but ParserFunctions also when error-ful data is input). Are errorbox styles suitably styled for inline use? (Given the name of box, I guess not.) They certainly don't look like it to me from the snapshot above.

I'd really prefer if parser-generated error classes were namespaced, eg mw-error instead of just error (if we're keeping that one).

While I agree, En.WP alone has 500 pages with custom CSS targeting .error (now I wonder what Monobook and Modern are doing). There's some cases where .mw-ui-vform is prefixing it otherwise most people are making it big, bold, and red, or all 3. (Take as many grains of salt for the fact we don't know how many of those custom pages are for editors who are active.)

The change in class name is trivial for editors with a basic knowledge of what they're doing, of course.

@cscott prefixing all code is on the long list :)
The first step is scoping this rule to .mw-parser-output and addressing any usages outside that one.

For now, any wikitext containing class="error" should continue to work as before. We can reconsider styling and class name later.

Change 682960 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Don't use p class="success" for success messages

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

There is a patch for CentralAuth that needs review https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CentralAuth/+/682973/ that is blocking resolving this task.
There is a code stewardship review for that extension at T252244,

I don't think this blocks us from moving on with removing legacy from skins, as the pages it styles do not seem to be business-critical.

Since a couple of days, elements marked as noticeable error are no longer appearing in red text color, with frame in red color where requested, in enlarged font size.

This is the case for:

  • error messages of various extensions and messages adjoined to system maintenance categories
  • error messages inserted by various gadgets on top of pages or in the footer region

The reason is obviously:

  • CSS code is no longer loaded for all pages
  • errors are permitted only within page content area, but not on top of page or in front of content

Steps to reproduce

Extension

  • Create the following wikitext:
<ref name="DoesNotExist" />
<references />
  • Preview source editing (Vector).
  • Expected: Error message with text in red colour and enlarged size.
  • What you get: Regular black colour in inherited size vanishing among the entire document.
    • Error message is marked by class="error".
    • No decoration included when delivered by server.

On Gadgets

Somebody might argue that all existing gadgets and user scripts shall be rewritten now and provide their own error decoration each via an additional CSS resource.

  • This would require a conflict free naming scheme of classes against the WMF classes (not all preceded by mw-, e.g. like those in the list above), not colliding with site class names of any involved wiki, or any other gadget.
  • Users might want to highlight errors more expressive, e.g. by applying a yellow background to the expected red colours, for all messages of all gadgets and extensions and any source.

MediaWiki Dictionary of Class Names

Where do I find the comprehensive list of all class names used by WMF,

  • telling me which are allowed in wikitext,
  • telling me which are allowed and supported for gadgets,
  • telling me which are not supported without further action by gadgets,
  • telling me which are not supported in wikitext?

Which is complete and a reliable promise which classes are to be used within about 1000 content projects, and which ones not?

Steps to reproduce

Extension

  • Create the following wikitext:
<ref name="DoesNotExist" />
<references />
  • Preview source editing (Vector).
  • Expected: Error message with text in red colour and enlarged size.
  • What you get: Regular black colour in inherited size vanishing among the entire document.
    • Error message is marked by class="error".
    • No decoration included when delivered by server.

I cannot reproduce this: https://www.mediawiki.org/wiki/User:Mainframe98/Sandbox#Phab:T280766 (nor on the beta cluster).

I cannot reproduce this: https://www.mediawiki.org/wiki/User:Mainframe98/Sandbox#Phab:T280766 (nor on the beta cluster).

I just double-checked this: German Wikipedia, German GUI, Vector(legacy). No CSS delivered by server.

Several other users are reporting the same and similar cases as well.

I still can't manage to reproduce - do you have a link to an existing example? I used Vector (legacy) and changed my language to German without any effect.

I cannot create articles occurring in such maintenance categories, nor other pages, since our maintenance team will remove that immediately.

When came the train into station? Those errors have been reported since a few days ago only, but from various users including myself. The annoncement was telling of June.

In Tech News: 2021-18 issued beginning of May 2021 I found now an inconspicious sentence amid of a longer statement:

The CSS classes .error, .warning and .success do not work for mobile readers if they have not been specifically defined on your wiki. From June they will not work for desktop readers. whaffle whaffle whaffle whaffle whaffle whaffle

  • Why was there no explicit statement:
    • Breaking Change
    • The classes .error, .warning and .success will be discontinued next year?
  • Why was no sufficient migration period given to migrate many scripts and gadgets? It is quite common practice to equip error messages globally and uniquely for two decades now. Why such a disruptive change immediately with no sufficient announcement?
  • To provide a globally unique appearance in all wikis (most gadgets are not limited to one particular wiki, but of universal use) – what is the mw.loader.load() module identifier to load the common wikistyle for error messages?
  • What has been the crucial need to remove the code immediately without leeway for migration?

It is okay to move in the long run to a .mw-error and .mw-errorbox naming scheme.

  • Gadget implementations might be changed one after the other and mw.load() a common module with unique design for all error cases.
  • It will take some years after Tech News publishing until all user scripts and gadgets adopt the new strategy.
  • User CSS customization is provided inwiki and per external sheets e.g. to put a yellow background behind everything with .error or .errorbox – how long do you expect that all users changed their styles available for two decades now?

Tech News: 2021-32 made it better for .visualClear

  • The message was clear and obvious.
  • Some three pages needed to be updated.
  • Fine.
  • No gadget programmer involved.

Not all JavaScript maintainers are permanently active and check every line and every word of every Tech News.

Change 682973 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Don't use .success and .error for success and error messages

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

Jdlrobson claimed this task.

Patching the other non-Wikimedia-deployed extensions is outside the scope of this ticket. The search URL throws up quite a few false positives anyway.