Page MenuHomePhabricator

Visual editor skin requirement not listed (#mw-content-subtitle)
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Use skin that not add any HTML element with the id #mw-content-subtitle
  • Try to save veaction=edit form

What happens?:
When finishing to save in save popup, in the last step (clicking "visualeditor-savedialog-title-save"button) - the popup just freezing

What should have happened instead?:

The popup would close

Software version (skip for WMF-hosted wikis like Wikipedia):

  • MW Version 1.40
  • Visual Editor from branch 1.40

Other information (browser name/version, screenshots, etc.):
The problem is because ve.init.mw.ArticleTarget.prototype.replacePageContent calling mw.util.addSubtitle which throwing error if no #mw-content-subtitle found.

It can be solved by code side, using catch or something - or #mw-content-subtitle requirment should be listed here

Event Timeline

matmarex added subscribers: Jdlrobson, matmarex.

Is it intended that mw.util.addSubtitle throws in this scenario?

It's expected that the #mw-content-subtitle element should always be present in the DOM. This should be true for all pages in Wikimedia skins even mobile main page (https://en.m.wikipedia.org/wiki/Main_Page). It's provided by Skin::prepareSubtitle which is made available for output by all skin templates (https://codesearch.wmcloud.org/search/?q=prepareSubtitle&files=&excludeFiles=&repos=).

That said there is an argument to be made given that 3rd party skins can basically do anything they want that we could either:

  1. check for the presence of this element and throw a more explicit error, but the code using it would still need to be adapted to handle it.
  2. add validation/tests to skins to make sure they output this element and/or can't call this function without it.

Use skin that not add any HTML element with the id #mw-content-subtitle

Which skins are you specifically reporting this issue for? Or is something removing this element that shouldn't be (if the latter please file a bug against that code to hide the element rather than destroy it)?

We have a custom skin is based on SkinMustache, but allows full customization of MediaWiki's appearance, including the option to remove the subtitle if needed.

The quickest fix, as suggested, might be to add #mw-content-subtitle to the skin requirements in the VisualEditor documentation.

I was getting this bug with the skin Freo (now fixed).

Adding as a requirement sounds good, that'd have saved me time (the VE docs about skin support were the first thing I checked). Throwing an error like it does for other required elements would be good (there obviously aren't that many skins that suffer from this).

I don't think we need to list a requirement -- I'm just going to add a patch to catch that error in the other ticket. We don't really depend on this existing, we're just passing on whatever the API has given us, so if the skin doesn't support subtitles we might as well just throw it away and continue.

DLynch claimed this task.

We removed the throw from mw.util.addSubtitle, so VE now has no need for this requirement. Skins that lack a properly-specified subtitle will fail to have it update, but they can easily fix that if they choose to..