Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Newlines get ignored #125

Open
richardmtsr opened this issue Feb 10, 2022 · 4 comments
Open

Newlines get ignored #125

richardmtsr opened this issue Feb 10, 2022 · 4 comments
Assignees
Labels
bug Something isn't working P1 Moderate Issue

Comments

@richardmtsr
Copy link
Contributor

Any newline character seems to be ignored, leaving my content rendered all on a single line. The conversion to react / html components is working but due to the ignoring of new lines it doesn't display correctly.

Is there a setting for this?

@JiLiZART
Copy link
Owner

Seems like a bug

@JiLiZART JiLiZART added the bug Something isn't working label May 9, 2022
@JiLiZART JiLiZART self-assigned this Sep 28, 2023
@JiLiZART JiLiZART added the P1 Moderate Issue label Sep 28, 2023
@mmichaelis
Copy link

As we also struggle with this, some thoughts here:

Architectural Challenge

With only having scratched the very surface of BBob, I see an architectural challenge in mapping, as there does not seem to be a "non-BBCode-tag" to element mapping approach. We would have some plain text content here, that in some contexts (thus, non-code-/non-preformatted-contexts) should get newlines transformed to an element.

<p> or <br>

The leading question for us was, if to wrap newlines into paragraphs and/or add <br> elements.

Unfortunately, there is a wild discussion in the net, which to prefer.

Some refer to behave similar to Markdown: Ignore newlines, unless either two in a row, which become a paragraph or, a newline after two spaces as last characters in a line that becomes a <br>.

Other solutions I found prefer plain newline to <br> mapping.

Conclusion of the Research

As it seems, most of the resources do not make up their mind on proper transformation of BBCode to HTML and vice versa when it comes to <br> and <p>. Rough tendency, without clear specification, seems to be to make any newline a <br> tag.

I would recommend adding some configuration options here, either by some "selected options" or to make it part of the preset-API how to deal with it.

Research

Some possible demo references:

And some demo references doing it the other way round (HTML to BBCode)

And some more links:

@JiLiZART JiLiZART added P0 Critical Issue and removed P1 Moderate Issue labels Oct 16, 2023
mmichaelis added a commit to CoreMedia/ckeditor-plugins that referenced this issue Oct 19, 2023
To address JiLiZART/BBob#125 diving somewhat
deeper into the BBob API. Assumption is, that the best place to
respect paragraphs is the `process` method, as we require dedicated
handling of text nodes and must not take them as is.

The current state is more of "inlining the HTML5-Preset", so that we
may start with experiments.
mmichaelis added a commit to CoreMedia/ckeditor-plugins that referenced this issue Oct 19, 2023
To address JiLiZART/BBob#125 adding some
utility method to handle contents of a tag and respect paragraphs
within.
@Alteras1
Copy link
Contributor

Thought I'd add my team's work to the discussion in case someone wants to go down the <br> route and save them a day's work.

We created a plugin that will walk the tree and insert <br> wherever there's an end of line. Our use case is a bit odd since we plan on using it alongside a bbcode tag [nobr] that will prevent the conversion of line breaks. Regardless, the code for the plugin can be generalized.

https://github.com/RpNation/bbcode/blob/main/bbcode-src/plugins/lineBreak.js

/**
 * Plugin that converts line breaks to `<br/>` tags.
 * To use, put as function similar to the presets.
 *
 * If a node is marked with `noLineBreakConversion`, then it'll skip the parsing the children
 *
 * @example
 * ```ts
 * const output = bbob([preset(), lineBreakPlugin()]).process(input, {render}).html
 * ```
 */
import { isEOL } from "@bbob/plugin-helper";

/**
 * Checks if input is an object
 * @param value input
 * @returns if value is an object
 */
const isObj = (value) => typeof value === "object";

/**
 * Walks the tree of nodes. Will add `br` tag to all `\n` in format that can be used in any renderer.
 * Preserves \n so that markdown-it doesn't try to treat everything like a block
 *
 * If a node has the property noLineBreakConversion is encountered, will skip parsing children.
 * @param t tree of nodes to be processed
 * @returns modified tree
 */
const walk = (t) => {
  const tree = t;

  if (Array.isArray(tree)) {
    for (let idx = 0; idx < tree.length; idx++) {
      const child = walk(tree[idx]);
      if (Array.isArray(child)) {
        tree.splice(idx, 1, ...child);
        idx += child.length - 1;
      } else {
        tree[idx] = child;
      }
    }
  } else if (tree && isObj(tree) && tree.content) {
    if (tree.disableLineBreakConversion) {
      // stop walk. children won't be parsed to have <br>
      return tree.tag ? tree : tree.content;
    }
    walk(tree.content);
  }

  if (isEOL(tree)) {
    return [{ tag: "br", content: null }, "\n"];
  }

  return tree;
};

/**
 * Converts `\n` to `<br/>` self closing tag. Supply this as the last plugin in the preset lists
 *
 * @example converts all line breaks to br
 * ```ts
 * const output = bbob([preset(), lineBreakPlugin()]).process(input, {render}).html
 * ```
 * @example will not convert line breaks inside [nobr]
 * ```ts
 * const nobr = (node: TagNode) => {return { disableLineBreakConversion: true, content: node.content }}; \\ tag in preset
 * ...
 * const output = bbob([preset(), lineBreakPlugin()]).process(input, {render}).html
 * ```
 * @returns plugin to be used in BBob process
 */
export const lineBreakPlugin = () => {
  return (tree) => walk(tree);
};

We made the deliberate choice to preserve \n alongside the <br> since our use case is a bit extreme where we need to make sure it doesn't break a downstream processing. Personally, I think this is better since the resulting HTML will be close to how <br> usage is. Either way, the return can be easily edited to something else if need be.

We just call the plugin after the preset. The intention is to run this as the final step before rendering.
https://github.com/RpNation/bbcode/blob/main/bbcode-src/index.js#L19-L22

  bbob([preset(), lineBreakPlugin()]).process(code, {
    render,
    ...options,
  });

So as a bit of background, we're using BBob as our future bbcode engine for migration from Xenforo, which also does the <br> replacement route. The extra downstream processing is actually a markdown-it engine that we modified to prevent <p> tag output.

I think this is enough for fulfilling the <br> replacement option?

@mmichaelis
Copy link

@Alteras1, thanks for your workaround for <br> handling via extra plugin. We thought of a similar approach, as this is an obvious extension point.

We skipped though, as we need the result to be parsed as CKEditor 5 data view, which again works best having paragraphs instead. And this again requires more "context awareness" best represented in a preset.

Our solution is a custom preset, mostly a copy of the HTML5 Preset (thus, we reuse the defaultTags). This is, because we need to be context aware (e.g., do not add paragraphs within a code block).

Our solution mainly consists of these adaptations:

  • process: Instead of the default-processor, that just walks the tree, we required an artificial root tag as intermediate state, so that we have "paragraph detection" on root level. Our customized function adds a [root] tag and later removes this prior to generating HTML.

  • Rules per Paragraph Support: For each TagNode we want to support paragraph parsing within its direct contents, we added an extra rule (thus, for example, for td, th and quote - also for li as this is the result of the default list processing). Benefit: Within inline tags, we simply ignore newlines (just as before), as will get broken HtML5 otherwise (must not add a paragraph within a span). This also fixes an issue with the existing quote mapping for BBob, which just wraps everything into a paragraph - which is again invalid for lists, for example.

  • Markdown Like Paragraph Parsing: In an extra function, we do all the paragraph parsing. We required defining "block-level tags" that must not be wrapped into paragraphs for valid HTML5. As a result, the following BBCode:

    [quote] Hello World!
    [list]
    [*] Item 1
    [/list]
    [/quote]
    

    would render to:

    <blockquote>
      <p>Hello World>!</p>
      <ul><!-- ... ---></ul>
    </blockquote>

    while the original HTML5 preset would have generated:

    <blockquote>
      <p>
        Hello World>!
        <ul><!-- ... ---></ul>
      </p>
    </blockquote>

Find more details in current snapshots:

Or as part of this pull request, introducing BBCode support as CKEditor 5 Plugin: CoreMedia/ckeditor-plugins#158 (possibly including more refactorings, as the PR is still in progress).

Intermediate Conclusion: Whatever the solution for this issue will be - we obviously need customization options depending on who is the consumer of the generated HTML.

@JiLiZART JiLiZART added P1 Moderate Issue and removed P0 Critical Issue labels Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1 Moderate Issue
Projects
None yet
Development

No branches or pull requests

4 participants