Page MenuHomePhabricator
Create Task
Maniphest T201573

Generalize logic for inserting a block level element into an empty paragraph (it should replace that paragraph, not insert before it)
Closed, ResolvedPublic
Actions

Description

Follow-up to T199925 and T201472.

In T201472#4488623, @Esanders wrote:

(…) I think ultimately we should move the paragraph cleanup into the command or action code:

If I type abc<newline>{| I expect to get <p>abc</p><table>..., which your previous patch achieves.

However if I type abc<newline> then choose Insert>Table, I don't get the benefits of the newline cleanup, which I think you should.

In general, taking any action to insert a block level element into an empty paragraph should destroy that paragraph.

In T201472#4488922, @DLynch wrote:

@Esanders: Makes sense, and I agree on the general case. If you merge it, I can make a new task for fixing it up more permanently.

Details

ProjectBranchLines +/-Subject
mediawiki/extensions/VisualEditormaster+8 -1dm.MWTableNode: include paragraph in suggestedParentNodeTypes
mediawiki/extensions/VisualEditormaster+1 -1Update VE core submodule to master (e2ce88ff1)
VisualEditor/VisualEditormaster+13 -3ve.ui.Sequence: Strip sequence before running the command
VisualEditor/VisualEditormaster+1 -18ve.ui.Sequence: Revert sequence stripping changes
VisualEditor/VisualEditormaster+14 -8ve.dm.Document: Replace empty nodes after inserting non-text data
Customize query in gerrit

Related Objects

Event Timeline

matmarex created this task.Aug 9 2018, 12:44 AM
Restricted Application added a subscriber: • Aklapper. · View Herald TranscriptAug 9 2018, 12:44 AM
matmarex mentioned this in T201472: List insertion by typing '#<space>', '*<space>' is broken.Aug 9 2018, 12:48 AM
matmarex added a comment.Aug 9 2018, 1:16 AM

This probably should be low priority… but it was interesting, so I investigated it for a bit, and ended up with working patches… so might as well submit them.

gerritbot added a subscriber: gerritbot.Aug 9 2018, 1:17 AM

Change 451552 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.dm.Document: Replace empty nodes after inserting non-text data

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

gerritbot added a project: Patch-For-Review.Aug 9 2018, 1:17 AM

Change 451553 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.ui.Sequence: Revert sequence stripping changes

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

gerritbot added a comment.Aug 9 2018, 1:17 AM

Change 451554 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] ve.ui.Sequence: Strip sequence before running the command

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

gerritbot added a comment.Aug 9 2018, 1:58 PM

Change 451552 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ve.dm.Document: Replace empty nodes after inserting non-text data

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

gerritbot added a comment.Aug 9 2018, 5:07 PM

Change 451553 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ve.ui.Sequence: Revert sequence stripping changes

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

gerritbot added a comment.Aug 10 2018, 11:28 AM

Change 451554 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] ve.ui.Sequence: Strip sequence before running the command

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

Jdforrester-WMF edited projects, added VisualEditor-EditingTools, VisualEditor (Current work); removed Patch-For-Review, VisualEditor.Aug 10 2018, 5:00 PM
Jdforrester-WMF added a project: VisualEditor-DataModel.
gerritbot added a comment.Aug 10 2018, 5:06 PM

Change 451880 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e2ce88ff1)

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

gerritbot added a project: Patch-For-Review.Aug 10 2018, 5:06 PM
Jdforrester-WMF moved this task from Incoming to Code review on the VisualEditor (Current work) board.Aug 10 2018, 5:29 PM
Jdforrester-WMF moved this task from Code review to QA on the VisualEditor (Current work) board.Aug 10 2018, 6:06 PM
Jdforrester-WMF edited projects, added MW-1.32-notes (WMF-deploy-2018-08-14 (1.32.0-wmf.17)); removed Patch-For-Review.
gerritbot added a comment.Aug 10 2018, 6:29 PM

Change 451880 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e2ce88ff1)

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

Jdforrester-WMF edited projects, added MW-1.32-notes (WMF-deploy-2018-08-21 (1.32.0-wmf.18)); removed MW-1.32-notes (WMF-deploy-2018-08-14 (1.32.0-wmf.17)).Aug 14 2018, 8:54 PM
Deskana triaged this task as Medium priority.Aug 17 2018, 9:28 AM
DLynch added a comment.Aug 23 2018, 5:10 PM

I observe that with the current patches, insert-table still leaves an empty paragraph (either via sequence or menu action).

Esanders added a comment.Aug 23 2018, 5:21 PM

I note that is true in MW, but not core.

Esanders added a comment.Aug 23 2018, 5:26 PM

It looks like ve.dm.MWTableNode.static.suggestedParentNodeTypes is the culprit. The fact 'paragraph' isn't in that list is probably causing the table to get moved down earlier on in the process (during fixupInsertion?)

Ryasmeen added a subscriber: Ryasmeen.Sep 1 2018, 12:09 AM

Looks like this is still happening, and should not be in QA column. Moving it to "Up Next" column.

Ryasmeen moved this task from QA to Incoming on the VisualEditor (Current work) board.Sep 1 2018, 12:09 AM
Esanders added a comment.Sep 4 2018, 12:27 PM

Pinging @matmarex , @DLynch

gerritbot added a comment.Sep 6 2018, 4:47 PM

Change 458534 had a related patch set uploaded (by DLynch; owner: DLynch):
[mediawiki/extensions/VisualEditor@master] dm.MWTableNode: include paragraph in suggestedParentNodeTypes

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

gerritbot added a project: Patch-For-Review.Sep 6 2018, 4:47 PM
DLynch moved this task from Incoming to Code review on the VisualEditor (Current work) board.Sep 6 2018, 5:10 PM
Esanders assigned this task to DLynch.Sep 7 2018, 12:53 PM
Esanders moved this task from Code review to In progress on the VisualEditor (Current work) board.Sep 13 2018, 11:48 AM
gerritbot added a comment.Sep 17 2018, 8:14 PM

Change 458534 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] dm.MWTableNode: include paragraph in suggestedParentNodeTypes

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

Jdforrester-WMF removed a project: Patch-For-Review.Sep 17 2018, 8:25 PM
Jdforrester-WMF moved this task from In progress to QA on the VisualEditor (Current work) board.
ReleaseTaggerBot edited projects, added MW-1.32-notes (WMF-deploy-2018-09-18 (1.32.0-wmf.22)); removed MW-1.32-notes (WMF-deploy-2018-08-21 (1.32.0-wmf.18)).Sep 17 2018, 9:00 PM
Ryasmeen added a project: Verified.Sep 18 2018, 2:20 AM
Ryasmeen moved this task from QA to Product owner review on the VisualEditor (Current work) board.
Deskana closed this task as Resolved.Oct 1 2018, 1:09 PM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptOct 1 2018, 1:09 PM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL