Page MenuHomePhabricator

Comments in "unsafe" content locations are not displayed in VisualEditor
Closed, ResolvedPublic40 Estimated Story Points

Description

See the 6 hidden comments I placed in https://en.wikipedia.org/w/index.php?title=User:Elitre_(WMF)/SandboxVE&action=edit .
Only 2 of them are visible when VEditing, https://en.wikipedia.org/wiki/User:Elitre_(WMF)/SandboxVE?veaction=edit .
Reported on fr.wp, see for example that they are missing from sections Personagges and Liens externes in https://fr.wikipedia.org/w/index.php?title=Détective_Conan&veaction=edit .


Version: unspecified
Severity: minor

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 3:59 AM
bzimport set Reference to bz71085.

The reason for this is that we're currently very conservative about which comments we deem to be "safe" to display, because there are some "unsafe" ones, like comments placed between two table rows. We haven't yet refined the logic for this, so right now a lot of comments are erroneously identified as unsafe.

Am I missing something obvious, or is this actually pretty simple?

The logic for this behavior is in Converter and ve.dm.CommentNode.static.toDataElement, we only generate a content node if converter.isExpectingContent() (=we're inside one of BlockquoteNode, HeadingNode, ParagraphNode, PreformattedNode, and expecting text). Therefore no comment will appear in VE for comments placed between block-level elements – paragraphs, tables, floated images, box-like templates, headings – and more confusingly, for comments placed at the beginning of a line containing a paragraph, for example (this could be blamed on Parsoid's funny output, though – the <!-- --> is outside the <p></p>) or at the beginning of a table cells and list items (and this is all our fault).

But! we already have logic to control which elements can be nested inside which (for example, table rows may only contain table cells; ordered list may only contain list items). And we already have logic to wrap things that are not block-level in paragraphs. Looks like we could easily use that.

I implemented it, played with it, and encountered no problems. Patch incoming.

Change 192051 had a related patch set uploaded (by Bartosz Dziewoński):
Generate CommentNodes more leniently

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

Patch-For-Review

matmarex added a project: Patch-For-Review.

The patch above correctly displays all six comments in the example from task description.

(this could be blamed on Parsoid's funny output, though – the <!-- --> is outside the <p></p>)

Filed as T90321.

Change 192051 merged by jenkins-bot:
Generate CommentNodes more leniently

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

Comment added after citation node is not getting added , nothing in review changes section after adding it.

Also , comment is not getting added after template,gallery,math node and table

@Ryasmeen, can you elaborate? I can't reproduce that, neither in labs nor prod – I can open an existing article in VE, place the cursor after a citation (like "[1]"), insert a comment there, and save the page with the comment appearing in the source code (I didn't actually save, but "Review your changes" shows it). There is currently a separate issue with inserting things into slugs (T89192) – are you sure you're not running into that?

Hmm, I am able to reproduce it consistently.Nope thats a different case, I am not adding anything into a slug.Attaching the screen capture if it helps

Ooh, thank you, that was helpful. I can reproduce only if the reference is at the end of paragraph, and you add the comment after it. Adding a comment before a reference, or adding it after and typing some more text, works correctly. I can also reproduce in production, so that must be a separate issue too.

Jdforrester-WMF renamed this task from VisualEditor: Comments in "unsafe" content locations are not displayed to Comments in "unsafe" content locations are not displayed in VisualEditor.Feb 25 2015, 11:38 PM
Jdforrester-WMF closed this task as Resolved.
Jdforrester-WMF moved this task from Accepted to Done on the VisualEditor 2014/15 Q3 blockers board.