Page MenuHomePhabricator

mw-empty-elt element is not hidden in old Vector
Closed, ResolvedPublic2 Estimated Story Points

Description

Empty list items should be hidden. When not hidden they can appear like this:


Example page: https://en.wiktionary.org/wiki/-ar

When we disabled the content parser output module in Vector because of T279008 we inadvertently removed this important rule.

Event Timeline

Change 679822 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Adjust floating override

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

ovasileva set the point value for this task to 2.Thu, Apr 15, 5:21 PM
Reedy renamed this task from mw-empty-elt element is not hidden in old Vector. to mw-empty-elt element is not hidden in old Vector.Thu, Apr 15, 6:24 PM

Change 679822 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Adjust floating override

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

Change 679842 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@wmf/1.37.0-wmf.1] Adjust floating override

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

Change 679842 merged by jenkins-bot:

[mediawiki/skins/Vector@wmf/1.37.0-wmf.1] Adjust floating override

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

Mentioned in SAL (#wikimedia-operations) [2021-04-15T23:35:52Z] <jforrester@deploy1002> Synchronized php-1.37.0-wmf.1/skins/Vector/resources/skins.vector.styles.legacy/layouts/screen.less: Backport: [[gerrit:679842|Adjust floating override (T280260)]] (duration: 00m 56s)

Mentioned in SAL (#wikimedia-operations) [2021-04-15T23:37:31Z] <jforrester@deploy1002> Synchronized php-1.37.0-wmf.1/skins/Vector/skin.json: Backport: [[gerrit:679842|Adjust floating override (T280260)]] (duration: 00m 56s)

Krinkle reopened this task as Open.EditedFri, Apr 16, 12:02 AM
Krinkle added a subscriber: Krinkle.

@Jdlrobson The commit doesn't appear to have had the intended effect, re-introducing T279008:

The added rule that you meant as override is shipped before the broken rule that it re-enabled. (about 500 lines between the two in the SkinModule response)

Perhaps it would be easier to add the one line of code from this new core rule that we actually need instead of shipping multiple copies of the same thing that are meant to cancel out (but didn't). Or since none of all these were meant to have any noticable affect on the user, to restore to the last-working state until there is time to research it better whilst master and production are not used as test bed. This is the third week in a row that the same refactor once again didn't work out as expected from causes that were rather predictable.

Jdlrobson closed this task as Resolved.EditedFri, Apr 16, 12:13 AM

Thanks for flagging but the issue here with the elt element is fixed.

I'd rather prioritize T279388 and get this done properly so I've moved that into the sprint board. As these last few weeks have proved it's not worth rushing a temporary fix, particularly during a time when this team is under pressure from multiple angles for a bug that this team does not recognize as unbreak now.

Note: in case it wasn't clear the ship sailed a long time ago (before the last version) for us to be able to restore the last working state due to the extension/skin/core relationship and at this point identifying and reverting squashes of all the patches involved here feels like the same amount, if not more work than actually taking the time to fix this properly. Currently, T270027 is higher priority than this hence the delay in my ability to focus on this.