Page MenuHomePhabricator

Existing categories that are preceded by new/modified content should be forced onto a new line
Closed, ResolvedPublic

Description

See https://he.wikipedia.org/w/index.php?diff=16380129 .. there the editor deleted a category. Likely the user or VE deleted the newlines as well and the serialized result is a bit ugly where the category now ended up on the same line as a list.

The serializer should be smarter and always force a category (existing / new) onto a new line if it is preceded by modified/new content. This lets parsoid not be overly sensitive on newlines that it receives from clients in some scenarios. This is similar to T72791.

Event Timeline

ssastry raised the priority of this task from to Medium.
ssastry updated the task description. (Show Details)
ssastry added a subscriber: ssastry.

Change 184707 had a related patch set uploaded (by Marcoil):
WIP: T86271: Add newline before category link

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

Patch-For-Review

I've pushed a WIP patch at https://gerrit.wikimedia.org/r/184707 , but I'm not sure if it's possible to fix this (very) specific situation without unintended consequences:

  • The code now has to recalculate separators whenever there's been deleted content between two nodes, which adds processing time. Also, the type of change (deleted, modified, etc.) now has to be tracked.
  • As a consequence, it messes the results of 25 selser tests which delete content. I'm afraid this is due to the separator handlers not taking into account that now they can be called in more occasions.
  • The conditions under which we want to insert the extra newline are really specific.

Wouldn't it be easier to have the editor program automatically insert a newline when deleting content between existing text and a category link?

I'll take a look at the patch and play with it .. but probably not right away since this is kind of lower priority.

But, a short answer is that as a stop-gap measure, it is fine requiring clients to insert those newlines, but, longer term, Parsoid's serialization should move towards being increasingly robust and handle these scenarios.

Change 239991 had a related patch set uploaded (by Subramanya Sastry):
[WIP] T86271: Serialize <link>s next to modified content on own line

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

ssastry set Security to None.

Change 184707 abandoned by Subramanya Sastry:
WIP: T86271: Add newline before category link

Reason:
in favour of https://gerrit.wikimedia.org/r/#/c/239991/

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

Change 239991 merged by jenkins-bot:
T86271: Serialize <link>s on own line always

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

Change 248946 had a related patch set uploaded (by Subramanya Sastry):
Revert "T86271: Serialize <link>s on own line always"

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

Change 248946 merged by jenkins-bot:
Revert "T86271: Serialize <link>s on own line always"

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

Change 249962 had a related patch set uploaded (by Subramanya Sastry):
WIP: Take #2: T86271: Serialize <link>s on own line always

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

What's the phab task # for the \u200e - \u200f RTL/LTR directionality character issue?

(Never mind, the diff in the description of this task demonstrates the problem.)

Change 249962 merged by jenkins-bot:
Take #2: T86271: Serialize <link>s on own line always

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