VisualEditor doesn't cope gracefully with line-breaking whitespace in a ref name
Closed, ResolvedPublic
Actions

Description

See the very informative comments at https://www.mediawiki.org/wiki/Topic:Vj7ufcyqarqji1cx by @Jhertel and @Weblars. Until you make an edit such as https://da.wikipedia.org/w/index.php?diff=10299554&oldid=10293564 to remove these characters from the ref name (which you can't do in the visual editor right now), then the visual editor won't save the page.

Testing

Visit this page with the console open: https://da.wikipedia.org/w/index.php?title=M/S_Scandinavian_Star&oldid=10290531&veaction=edit
Type some extra text anywhere in the document
Attempt to switch to wikitext

Obervered: An exception is thrown, switching fails
Expected: No exception thrown, switching succeeds

Details

	Subject	Repo	Branch	Lines +/-
	Match line-breaking whitespace in reference names	mediawiki/extensions/Cite	master	+5 -5

Customize query in gerrit

Related Objects

Mentioned In: T267974: Some linebreak chars in <ref>'s name attribute (and possibly other attributes as well) not handled correctly by Parsoid's Cite implementation

Event Timeline

• Whatamidoing-WMF created this task.Sep 23 2020, 10:21 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 23 2020, 10:21 PM

The character ends up in the JS as "\u2028" and breaks the regex:

'literal/foo\u2028bar'.match(/literal\/.*/)
> ["literal/foo", index: 0, input: "literal/foo bar", groups: undefined]

Note that the .* stops matching after foo

Esanders renamed this task from VisualEditor doesn't cope gracefully with ö and similar characters in a ref name to VisualEditor doesn't cope gracefully with line-breaking whitespace in a ref name.Sep 24 2020, 12:07 PM

Change 629670 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/Cite@master] Match line-breaking whitespace in reference names

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

gerritbot added a project: Patch-For-Review.Sep 24 2020, 12:15 PM

Esanders claimed this task.Sep 24 2020, 12:15 PM

Esanders updated the task description. (Show Details)

Esanders moved this task from To Triage to Triaged on the VisualEditor board.

Esanders added a project: Editing-team (Kanban Board).

Esanders moved this task from Incoming to Code Review on the Editing-team (Kanban Board) board.

There's probably a Parsoid issue here. If you type a regular newline in a ref name, it's replaced with a space when parsing – the same should probably happen to this character, and any similar newline-like things (probably just U+2028 and U+2029?). But it won't hurt to also handle this in VE.

test<ref name="a
b">asdf</ref>

blah<ref name="a
b" />

test<ref name="a b">qwer</ref>

blah<ref name="a b" />

<references />

Exception for reference, in case this comes up in error logging or someone files duplicates:

Firefox:

Uncaught TypeError: listKeyParts is null
load.php:669:816
    toDomElements jQuery
    getDomElementsFromDataElement ...:313
    getDomSubtreeFromData ...:337
    getDomSubtreeFromModel ...:329
    getDomFromModel ...:328
    getDom ...:227
    getDom ...:657
    createDocToSave ...:19
    getDocToSave ...:19
    getWikitextDataPromiseForDoc ...:36
    switchToWikitextEditor ...:36
    switchToWikitextEditor ...:952
    editSource ...:19
    switch ...:48
    onSelect ...:1553
    jQuery 4
    <anonymous> self-hosted:935
    jQuery 2
    <anonymous> self-hosted:935
    jQuery 15
    setup ...:681
    setupToolbar ...:67
    setupToolbar ...:29
    setupToolbar ...:926
    setSurface ...:66
    setSurface ...:1043
    setSurface ...:931
    addSurface ...:1043
    addSurface ...:925
    setupSurface ...:1042
    (Async: setTimeout handler)
    setupSurface ...:1042
    (Async: setTimeout handler)
    setupSurface ...:1042
    documentReady ...:1040
    documentReady ...:8
    loadSuccess ...:6
    loadSuccess ...:928
    <anonymous> self-hosted:935
    jQuery 2
    load ...:18
    activate ...:930
    activateTarget ...:13
    jQuery 19

Chrome:

Uncaught TypeError: Cannot read property '1' of null
    at Object.ve.dm.MWReferenceNode.static.toDomElements (...:669)
    at VeDmConverter.ve.dm.Converter.getDomElementsFromDataElement (...:313)
    at VeDmConverter.ve.dm.Converter.getDomSubtreeFromData (...:337)
    at VeDmConverter.ve.dm.Converter.getDomSubtreeFromModel (...:329)
    at VeDmConverter.ve.dm.Converter.getDomFromModel (...:328)
    at VeDmSurface.ve.dm.Surface.getDom (...:227)
    at VeUiMWSurface.ve.ui.Surface.getDom (...:657)
    at VeInitMwDesktopArticleTarget.ve.init.mw.ArticleTarget.createDocToSave (...:19)
    at VeInitMwDesktopArticleTarget.ve.init.mw.ArticleTarget.getDocToSave (...:19)
    at VeInitMwDesktopArticleTarget.ve.init.mw.ArticleTarget.getWikitextDataPromiseForDoc (...:36)

Change 629670 merged by jenkins-bot:
[mediawiki/extensions/Cite@master] Match line-breaking whitespace in reference names

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

matmarex moved this task from Code Review to QA on the Editing-team (Kanban Board) board.Sep 24 2020, 7:11 PM

matmarex edited projects, added Editing QA; removed Patch-For-Review.

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.11; 2020-09-29).Sep 24 2020, 8:00 PM

Is this resolved on the VE side? I will open a separate ticket for T263698#6492289.

ssastry mentioned this in T267974: Some linebreak chars in <ref>'s name attribute (and possibly other attributes as well) not handled correctly by Parsoid's Cite implementation.Nov 16 2020, 9:28 PM

ssastry removed a project: Parsoid.Nov 16 2020, 9:30 PM

matmarex moved this task from QA to Ready for Sign Off on the Editing-team (Kanban Board) board.Nov 16 2020, 9:37 PM

matmarex edited projects, added Skipped QA; removed Editing QA.

ppelberg closed this task as Resolved.Nov 21 2020, 3:02 AM

Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptNov 21 2020, 3:02 AM