Page MenuHomePhabricator

Investigate relationship of book referencing project and parsers
Open, Needs TriagePublic13 Story PointsSpike

Description

Motivation
So far, we were always under the assumption that for book referencing to work, we would need to make changes to the parser. Now it turns out this might not be true. At least for the non-VE part of things, only changes to the Cite Extensions are needed.

Acceptance Criteria
Find out:

Event Timeline

WMDE-Fisch updated the task description. (Show Details)Aug 21 2019, 12:44 PM
awight updated the task description. (Show Details)Aug 21 2019, 12:45 PM
WMDE-Fisch set the point value for this task to 13.Aug 21 2019, 12:48 PM

What happens if a page with book references is opened with the VisualEditor

Using the current PHP only PoC patch[1], the VE cannot make sense out of the refines attribute in the ref tag and introduces new footnote entries for these tags in the WYSIWYG preview :-(

Wikitext used:

1Lorem ipsum dolor it amet<ref name="foo">This is as book</ref>, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat<ref name="nope">No book here</ref>. Duis aute irure dolor in reprehenderit is voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur<ref refines="foo" name="bar">page 10</ref> sint occaecat cupidatat non proident, sunt in culpa qui officia<ref refines="foo">page 108 ff</ref> deserunt mollit anim id est laborum<ref name="bar" /><ref name="nope" />.

[1] https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Cite/+/530399/5

WMDE-Fisch updated the task description. (Show Details)Aug 22 2019, 9:30 AM
awight updated the task description. (Show Details)Aug 27 2019, 11:48 AM
awight added a subscriber: awight.Aug 27 2019, 2:53 PM

Looking at rollback scenarios,

  • A naive revert will result in broken wikitext and dramatic parser errors.
  • The minimum sane rollback strategy is that we continue to allow the refines attribute in ref tags, but they lose some of their new behavior. This prevents a parser error, but sub-references will be rendered without any indication of their parent context.
  • If the rollback will be permanent, we should rewrite any affected references to... maybe copy and prepend the parent context into the subreference.
  • We would need to anticipate reasons for revert, to plan the downgrade migration.
    • If the objection might be just a UI thing, we could include a configurable compatibility mode which displays the subreference with the parent prepended.
    • If the objection is about the wikitext change, we'll be in a bad situation where we need to edit lots of affected pages.
awight updated the task description. (Show Details)Aug 27 2019, 2:55 PM

The rollback concerns currently qualify us for a technical RfC. I'll continue looking at alternatives.

awight added a subscriber: jkroll.Aug 29 2019, 7:42 AM

Rethinking my concerns, any forward-incompatibilities due to the refines attribute would be entirely local to the Cite extension. We can write our implementation in two parts, in the first stage this new attribute is allowed but has no effect. In the second stage we conditionally render the subreferences when this attribute is present, if a feature flag is turned on. This gives us two partial rollback mechanisms which should both be safe: either disable the feature flag, or revert only the second patch. In either rolled-back state, we will render the subreferences badly and that will be the limit of the breakage.

We can also provide a fallback implementation which doesn't perform hierarchical organization of the subreferences but instead prepends the parent reference, maybe with a bit of formatting. This should give us a non-broken rolled-back state in case the hierarchical rendering is buggy.

VisualEditor parsing might throw this plan out the window, of course. To match the above logic, we would have to support a partial rollback mode in which the refines attribute was allowed, but perhaps hidden from the editor. @jkroll This is something you might want to consider in your VE investigation.

awight assigned this task to jkroll.Aug 29 2019, 11:41 AM

Nothing came of my theory that we could also piggyback on the name attribute. Redefining a named reference is definitely not backwards-compatible.

jkroll added a comment.Sep 4 2019, 1:18 PM

Regarding the rollback:

This is what it would look like if we reverted only the JS VE-Reference-Refines stuff and left the PHP changes, as they exist now, in there. So this is with Change 530399 applied. Rendered page:

And this is what it looks like in visual editor:

Saving the page in VE does not seem to break existing <ref>s with refines.