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

Improve functioning of Config.cleanupWikifierOutput #22

Open
selden opened this issue Apr 18, 2020 · 7 comments
Open

Improve functioning of Config.cleanupWikifierOutput #22

selden opened this issue Apr 18, 2020 · 7 comments
Assignees
Labels
investigate Information gathering is needed todo Make this happen

Comments

@selden
Copy link

selden commented Apr 18, 2020

WAS: Problem: Automatic paragraph indentation is inconsistent


Problem: Automatic paragraph indentation is inconsistent

This problem is seen when using SugarCube v2.31.1 (compiled by tweego v2.1.1+81d1d71).

Since the symptoms are somewhat more complex than discussed in the IF Forum, I decided it would be appropriate to report this formally as a bug.

When a section of text consisting of multiple paragraphs is not enclosed by either a "span" or a "div", all paragraphs are properly indented.

When a section of text consisting of multiple paragraphs is enclosed by a "span", only the first paragraph is indented.

When a section of text consisting of multiple paragraphs is enclosed by a "div", none of the paragraphs are indented.

In all three cases, I believe that all paragraphs should be indented.

Here's a twee/SugarCube story which demonstrates the problem.

:: StoryData
{
        "ifid": "C31A5464-60FC-4937-A550-C7D75F7692AF"
}

:: StoryTitle
Test Paragraph Indentation

:: Story Stylesheet [stylesheet]

/* indent paragraphs (in conjunction with Config below) */
.passage p { margin-left: 1.0em;
	     margin-right: 1.0em;
	     text-indent: 2.0em;
	    }

:: Story JavaScript [script]

/* automatically indent paragraphs */
Config.cleanupWikifierOutput = true;

:: Start
!! This is the Start passage.

When simply included, all paragraphs are indented:

<<include "content">>
----
When enclosed in a "span", the first paragraph is indented:

<span id="first"> <<include "content">> </span>
----
When enclosed in a "div", no paragraphs are indented:

<div id="none"> <<include "content">> </div>

:: content

Magna illud minim ius at, pro no numquam expetendis, an eam vidit denique. No numquam detraxit eos, inani detracto te quo, ad vim tritani accommodare. Utamur insolens qui no, cum at accusata delicatissimi, unum albucius ne eam.

Magna illud minim ius at, pro no numquam expetendis, an eam vidit denique. No numquam detraxit eos, inani detracto te quo, ad vim tritani accommodare. Utamur insolens qui no, cum at accusata delicatissimi, unum albucius ne eam.

@greyelf
Copy link

greyelf commented Apr 18, 2020

The <p> (paragraph) element documentation states that that element can contain Phrasing content.

So it appears that the line that consists of a <span> element is valid inner content for a paragraph element, the line consisting of a <div> element is not.

@selden
Copy link
Author

selden commented Apr 18, 2020

It seems to me that that document is discussing things which can be included within a paragraph, not things that a paragraph can be included in. E.g. inside a paragraph you can have a span. In other words, it's valid to have <p> <span> </span> </p>

The bug that I'm trying to report, perhaps not sufficiently clearly, is that the processing invoked by

Config.cleanupWikifierOutput = true;

supposedly replaces all occurrences of two <br> directives by a <p>...</p> pair. Unfortunately, it doesn't always do that. As a result, one must (currently) manually insert the paragraph directives when those paragraphs are within a <div> ... </div>.

@tmedwards
Copy link
Owner

That's not entirely accurate. What Config.cleanupWikifierOutput is supposed to do when enabled is spelled out within its documentation.

Determines whether the output of the Wikifier is post-processed into more sane markup—i.e., where appropriate, it tries to transition the plethora of <br> elements into <p> elements.

Emphasis added. Meaning where allowed by the specification and when it can make sense of the node graph—the distinction is important, especially for the former.


It probably should be looking into <div> elements, but that's not something I can change in v2 as it would be a breaking change. I can, however, tack this onto v3's pile.

It should not be looking into <span> elements, because <p> elements are not allowed within <span> elements. The MDN, W3C, and WHATWG documentation may be confusing on this point, however, the basic gist is that, per the specification, block-level elements should not generally be children of text-level elements.

@tmedwards tmedwards self-assigned this Apr 18, 2020
@selden
Copy link
Author

selden commented Apr 18, 2020

If I'm understanding you correctly, you're saying that the bug is the indentation of a paragraph when it's been included inside a span. I can accept that.

I also can be patient and wait until v3.

For the short term, I'll try eliminating the inter-passage transition and going to another passage to view additional (included) content. That introduces a flicker, but it's less objectionable to me than manually inserting all the paragraph markers.

@tmedwards
Copy link
Owner

No. I'm saying that injecting <p> elements into a <span> would be a bug. I expect that if you check the inspector you'll find that your <span> was placed within a <p>, rather than the other way around.

@selden
Copy link
Author

selden commented Apr 19, 2020

Yup. You're right, of course.

I personally find it annoying that expressions which look the similar while editing produce quite different results when evaluated. Oh, well. The underlying language definition is what it is.

Thanks for the clarification.

@tmedwards tmedwards transferred this issue from tmedwards/sugarcube-2 Jul 30, 2020
@tmedwards tmedwards changed the title Problem: Automatic paragraph indentation is inconsistent Improve Config.cleanupWikifierOutput code Aug 2, 2020
@tmedwards tmedwards changed the title Improve Config.cleanupWikifierOutput code Improve functioning of Config.cleanupWikifierOutput Aug 2, 2020
@tmedwards tmedwards transferred this issue from another repository Aug 2, 2020
@tmedwards tmedwards added investigate Information gathering is needed todo Make this happen labels Aug 2, 2020
@tmedwards
Copy link
Owner

tmedwards commented Aug 2, 2020

SEE ALSO: tmedwards/sugarcube-2#61 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate Information gathering is needed todo Make this happen
Projects
None yet
Development

No branches or pull requests

3 participants