Page MenuHomePhabricator

Blockquotes aren't ContentBranchNodes in reality, and our code shouldn't try to insist that they are
Closed, ResolvedPublic

Description

This is valid HTML:

<blockquote>
<p>
Foo
</p>
<p>
Bar
</p>
</blockquote>

But when such structure is edited in VE, the inner paragraphs get alienated.
*sighs*

In T154231, @dchan wrote:

At present, BlockquoteNode behaves like a ContentBranchNode ; it should not.

Right now, ve.ce.BlockquoteNode extends ve.ce.ContentBranchNode, but ve.dm.BlockquoteNode extends ve.dm.BranchNode . I would say BranchNode is the correct parent, because HTML blockquote elements can contain flow content such as paragraph elements (see https://html.spec.whatwg.org/multipage/semantics.html#the-blockquote-element ).

If however there's some special weird reason to treat BlockquoteNode as a ContentBranchNode, then I'll implement that instead and document the special weird reason.

ISTR there is — or at least, was — a solid reason for doing this. But I don't recall at all what it was.

It seems the reason was that we treat it as a top-level block element, like paragraphs or heading.

This is probably correct but it means we have to come up with a new tool for inserting block quote, as it is currently treated as a CBN conversion in the format dropdown. I think we did it that way because most blockquotes are only one paragraph.

Alternatively, we could go the other way and make it fully ContentBranchNode, at least for the time being.

It was actually changed to be fully ContentBranchNode in 94dbb244dd.

Related Objects

Event Timeline

Jdforrester-WMF raised the priority of this task from to Medium.
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF moved this task to TR6: Visual diffs on the VisualEditor board.
Jdforrester-WMF changed Security from none to None.
Krenair renamed this task from Blockquotes aren't CBNs in reality, and our code shouldn't try to insist that they are to Blockquotes aren't ContentBranchNodes in reality, and our code shouldn't try to insist that they are.Dec 2 2014, 9:32 PM

Change 329448 had a related patch set uploaded (by Bartosz Dziewoński; owner: Divec):
[VisualEditor/VisualEditor@master] Make BlockquoteNode fully non-ContentBranchNode

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

Change 329448 abandoned by Divec:
Make BlockquoteNode fully non-ContentBranchNode

Reason:
We went the opposite way (fully ContentBranchNode) in 94dbb244dd

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

We can resurrect https://gerrit.wikimedia.org/r/329448 if we provide a tool for blockquote insertion

Change 475961 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] Make blockquote a non-content branch node, update tools and stuff accordingly

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

Change 475962 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update stuff for making blockquote a non-content branch node

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

cc @iamjessklein

The patch proposes moving the tool from "Format" to "Structure":

image.png (696×1 px, 80 KB)
image.png (696×1 px, 85 KB)

…and some interesting changes to how the tool behaves – blockquotes can now be nested in other nodes (e.g. lists) and have other nodes nested inside them (e.g. also lists, but more simply, paragraphs and headings etc.). They can also be nested in each other.

Open questions:

  • Should the tool remain in the "Format" menu? Probably not, as that would allow two items in that menu to be active at the same time, which looks silly.
  • Should Ctrl+8 remain as the keyboard shortcut? It doesn't really make sense anymore now that this is not the 8th item in the "Format" menu.
  • Should the tool have an icon? There is space for one now. I wanted to use 'quotes', but it has been usurped by Citoid.
  • Should we have some styles that make it easier to tell where a blockquote begins and ends, especially in case the user inserts multiple next to each other?

Copying replies from gerrit:

Should the tool remain in the "Format" menu? Probably not, ...

Agreed.

Should Ctrl+8 remain as the keyboard shortcut? ...

Meh. We have ctrl+shift+5 for strikethrough, and we short on available shortcuts

Should the tool have an icon? ...

Yeah - "quotes" is fine for now - if we don't like the duplication we can fix it later.

Should we have some styles that make it easier to tell where a blockquote begins and ends...

Possibly - they should be subtle/greyed out so it's clear they aren't part of the read mode styling, a bit like how we have default table cell styling. Some wikis do have blockquote styling though.

Re: changing the location of blockquotes: I know that @cmadeo did some kind of audit when she was look at editing app UIs. Carolyn - did you find the blockquotes functionality located in other places in the UI? (Other than nested under the dropdown for paragraph styles). I'd like to be a bit mindful before moving this because we are trying to maintain consistency as much as possible across the different interfaces.

Re: identifying an appropriate icon for blockquotes: In terms of icon, most folks have it as quotes (even phabricator). Medium has an interaction where you click the button multiple times and cycle through the different options for quote formatting, but I find that to be a touch unintuitive. I'm not sure what to use in terms of the icon though, I suspect that if we have multiple quote icons it could begin to complicate the issue. cc/ @Volker_E

Re: changing the location of blockquotes: I know that @cmadeo did some kind of audit when she was look at editing app UIs. Carolyn - did you find the block quotes functionality located in other places in the UI? (Other than nested under the dropdown for paragraph styles). I'd like to be a bit mindful before moving this because we are trying to maintain consistency as much as possible across the different interfaces.

Unfortunately block quote was not a common formatting option:

  • The Notion app places block quotes in with both what we consider paragraph stylings and list styles
  • The Medium app has block quotes as it's own toolbar level option (eg. not related to paragraph stylings)

Currently on the iOS app we're following suit with how VE is handling this, but I'm happy to move this out of paragraph styling if you all come to a different conclusion.

Not based on any testing, but I could see this falling under text stylings (same as computer code), but it does seem like block quote and preformatted text should be grouped together. Is this issue (alienated paragraphs) occurring with preformatted text from VE?

To the icon: Blockquote command is basically done in every Rich Text Editor GUI (email/web) with the beginning (English!) quotes “ that we use for “Cite” function in VE.
Gmail as example

image.png (40×38 px, 331 B)

We've prob locked ourselves out for using the same icons for those two different use cases, although I wonder how many new editors are confused by finding a functionality that the assume to know resulting in a different action.

An issue I'd careful with is putting the blockquotes underneath “list”. It's often close to the lists in other interfaces, but I wonder if people will actually assume or find it there. I wouldn't. Point taken, that it might be one of the least used features anyways.

Process question - is this something that we should focus on in conjunction with T211255?

moving this to Blocked/Waiting because I suspect that the work in https://phabricator.wikimedia.org/T211255 will impact the decisions on this.

Change 475961 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] Make blockquote a non-content branch node, update tools and stuff accordingly

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

Change 475962 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] [PULLTHROUGH] Update stuff for making blockquote a non-content branch node

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

The patches now only resolve the technical issue of multi-paragraph blockquotes not being editable. I'll file a separate task to consider the toolbar changes later.

Change 475961 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Make blockquote a non-content branch node

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

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

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

The patches now only resolve the technical issue of multi-paragraph blockquotes not being editable. I'll file a separate task to consider the toolbar changes later.

I filed: T221692: Consider moving the blockquote tool to another section of the toolbar

There is formal QA approval on T209162… I think we can skip the rest, things seem to work fine.