Page MenuHomePhabricator

VisualEditor incorrectly removes infobox yet preview does not reflect this
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. Go to https://en.wikipedia.org/wiki/Islamic_Exorcist.
  2. Go to the revision before I edited: https://en.wikipedia.org/w/index.php?title=Islamic_Exorcist&diff=840083704&oldid=838705119&diffmode=source.
  3. Edit page using VisualEditor.
  4. Paste this at the very top of the page: {{Distinguish|Exorcism in Islam}}
  5. Click the Publish changes... button.
  6. Click Review your changes.
  7. Notice how the Visual review does not show the removal of an infobox.
  8. Publish your changes.
  9. Notice how the infobox has been removed.

Had to edit in source mode to bypass this. You may refer to the page history to learn more.

Event Timeline

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

I don't have the time to check this myself right now, but I think this is similar/identical to T203879: When editing an old version of a page, the VisualEditor shows the diff to that old version and not to the most recent version?

That was my hypothesis as well, but both the revisions in question and the current revision have the infobox present, so I don't think it is? It's quite possible I'm misunderstanding as well. I'd appreciate it if you could double-check this for me, when you next get a chance. :-)

Deskana moved this task from To Triage to Needs Discussion/Analysis on the VisualEditor board.

Right, so, it actually is a separate issue. For some reason, pasting that wikitext in there does remove the infobox. It also happens if you paste the wikitext into the current revision: the visual editor shows the distinguish template twice with the infobox there, but the wikitext actually only shows a single newline being removed, with no other changes. This appears to be an oddity with the way that paste is working. Needs investigation to determine whether it's a bigger general problem, or whether it's some strangely specific issue.

Esanders subscribed.

Looks like an about-grouping issue. Templates can consist of adjacent sibling nodes if they have the same about attribute (e.g. <span about="#mwt1">One</span><span about="#mwt1">Template</span>). In this case the infobox on the page starts of with an about attribute of "#mwt1", and the pasted template is given the same attribute (about="#mwt1"). Parsoid then thinks the first two templates are in fact one template, and so ignores the infobox.

Change 460020 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Use data.cloneElements when converting pasted wikitext

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

Change 470503 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/Cite@master] Fix wikitext paste test

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

Change 460020 merged by Jforrester:
[mediawiki/extensions/VisualEditor@master] Use data.cloneElements when converting pasted wikitext

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

Jdforrester-WMF moved this task from Code review to QA on the VisualEditor (Current work) board.
Jdforrester-WMF subscribed.

Not a new shared build failure. Just the usual VE/Cite circular dependency that we've had for five years now.

Change 470503 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Fix wikitext paste test

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

No, it's not that one.

5 minutes after merging https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/VisualEditor/+/460020/, all commits to core and extensions are consistently 2-3 of their builds fail with the below error:

SUMMARY:
✔ 1420 tests completed
ℹ 5 tests skipped
✖ 1 test failed

FAILED TESTS:
  ve.ui.MWWikitextStringTransferHandler (Cite)
    ✖ convert
      HeadlessChrome 0.0.0 (Linux 0.0.0)
    Simple reference: data match
    Expected: [
      {
        "attributes": {
          "contentsUsed": true,
          "listGroup": "mwReference/",
          "listIndex": 0,
          "listKey": "auto/0",
          "mw": {
            "attrs": {},
            "body": {
              "id": "mw-reference-text-cite_note-1"
            },
            "name": "ref"
          },
          "originalMw": "{\"name\":\"ref\",\"body\":{\"id\":\"mw-reference-text-cite_note-1\"},\"attrs\":{}}",
          "refGroup": "",
          "refListItemId": "mw-reference-text-cite_note-1"
        },
        "type": "mwReference"
      },
      {
        "type": "/mwReference"
      },
      {
        "type": "internalList"
      },
      {
        "type": "internalItem"
      },
      {
        "internal": {
          "generated": "wrapper"
        },
        "type": "paragraph"
      },
      "F",
      "o",
      "o",
      {
        "type": "/paragraph"
      },
      {
        "type": "/internalItem"
      },
      {
        "type": "/internalList"
      }
    ]
    Actual: [
      {
        "attributes": {
          "listGroup": "mwReference/",
          "listIndex": 0,
          "listKey": "auto/0",
          "refGroup": "",
          "refListItemId": "mw-reference-text-cite_note-1"
        },
        "type": "mwReference"
      },
      {
        "type": "/mwReference"
      },
      {
        "type": "internalList"
      },
      {
        "type": "internalItem"
      },
      {
        "internal": {
          "generated": "wrapper"
        },
        "type": "paragraph"
      },
      "F",
      "o",
      "o",
      {
        "type": "/paragraph"
      },
      {
        "type": "/internalItem"
      },
      {
        "type": "/internalList"
      }
    ]
        at

This shouldn't be surprising because https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/VisualEditor/+/460020/ was already failing its own tests on Gerrit, and was forcefully merged without Jenkins passing.

The details of this new assertion failure are different from the "usual" VE/Cite flaky test, which is indeed old but fairly infrequent. This is happening on all commits.

Change 470503 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Fix wikitext paste test

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

Thanks :)

No, it's not that one.

Yes, it is. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Cite/+/470503 and https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/VisualEditor/+/460020/ depend on each other. It is still not possible for CI to merge them, so we have to bypass, as always.

5 minutes after merging https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/VisualEditor/+/460020/, all commits to core and extensions are consistently 2-3 of their builds fail with the below error

Obviously.

The details of this new assertion failure are different from the "usual" VE/Cite flaky test, which is indeed old but fairly infrequent. This is happening on all commits.

What flaky Cite test? We're not talking about a flaky test.

Checked on Beta cluster since the replication step requires saving the edit. Looks fixed on Beta cluster.