Page MenuHomePhabricator

Ignore <hr /> in <poem> when adding <br />
Closed, ResolvedPublic

Description

<poem>
lorem
ipsum
<hr />
dolor
sit
amet
</poem>

gives

<div class="poem">
<p>lorem<br />
ipsum<br /></p>
<hr />
<br />
<p>dolor<br />
sit<br />
amet</p>
</div>

but should render as

<div class="poem">
<p>lorem<br />
ipsum<br /></p>
<hr />
<p>dolor<br />
sit<br />
amet</p>
</div>

(note no <br /> after <hr />)

Rationale:
Poems use separators in them, such as *, * * *, etc... Such separators are just visual and semantically are supposed to be represented by <hr /> (with respective styling), not by plaintext. <br /> after <hr /> causes different spacing before and after the <hr />.

Event Timeline

Danny_B raised the priority of this task from to Medium.
Danny_B updated the task description. (Show Details)
Danny_B added projects: Cite, good first task.
Danny_B subscribed.

the same similar:

<poem>
lorem
ipsum
<div>loren</div>
dolor
sit
amet
</poem>

gives

<div class="poem">
<p>lorem<br />
ipsum<br /></p>
<div>loren</div>
<br />
<p>dolor<br />
sit<br />
amet</p>
</div>

(note <br /> after </div>)

AND we are using this feature during formatting poetry.

the same:

<poem>
lorem
ipsum
<div>loren</div>
dolor
sit
amet
</poem>

gives

<div class="poem">
<p>lorem<br />
ipsum<br /></p>
<div>loren</div>
<br />
<p>dolor<br />
sit<br />
amet</p>
</div>

(note <br /> after </div>)

AND we are using this feature during formatting poetry.

How is this comment relevant to this task, please?

Taking a tentative step back here, I have to ask -- why do we even need this at all? Why would a poem support having HTML at all in there, be it <hr /> or <div> or anything else (except for inline html like bold, italics, etc)

The entire point of the <poem> extension tag is to format the text as a poem visually. Why would we want an <hr> in there at all? Why would we want a block <div> ? And if we ever do need either of those, is it not wiser to split them out - for example:

<poem>
lorem
ipsum
</poem>
<hr/>
<poem>
dolor
sit
amet
</poem>

It seems to me that adding support for <hr/> may solve a problem that maybe shouldn't be used in the first place? Or am I missing use cases at all?

Taking a tentative step back here, I have to ask -- why do we even need this at all? Why would a poem support having HTML at all in there, be it <hr /> or <div> or anything else (except for inline html like bold, italics, etc)

The entire point of the <poem> extension tag is to format the text as a poem visually. Why would we want an <hr> in there at all? Why would we want a block <div> ?

Task description at its bottom clearly defines the rationale.

And if we ever do need either of those, is it not wiser to split them out - for example:

<poem>
lorem
ipsum
</poem>
<hr/>
<poem>
dolor
sit
amet
</poem>

No, it wouldn't.
With appropriate styling, you would then get two blocks of different widths and line across the entire page (or i.e. * * * centered over the entire page) in this case.
The separator needs to accomodate to the widest part of the poem which means it has to be within the poem block.

It seems to me that adding support for <hr/> may solve a problem that maybe shouldn't be used in the first place?

It should be used. The rationale in task description clearly states why. It's a question of semantics and accessibility.

Or am I missing use cases at all?

Yes you are, as described above... ;-)

Huh? Poem does not affect widths, it allows left hand spaces, and allows elimination of <br />, and stuffs up other internal tags.

If you want to affect widths, you would normally wrap within a centered block or the like. At enWS, the use of Template:*** works fine following a closed poem, and prior to a a new poem tag. Allows total control, and does not suppose anything about hard rules.

https://en.wikisource.org/w/index.php?title=Wikisource:Sandbox&diff=prev&oldid=6144443

I am in agreement with @Mooeypoo

Taking a tentative step back here, I have to ask -- why do we even need this at all? Why would a poem support having HTML at all in there, be it <hr /> or <div> or anything else (except for inline html like bold, italics, etc)

The entire point of the <poem> extension tag is to format the text as a poem visually. Why would we want an <hr> in there at all? Why would we want a block <div> ?

Task description at its bottom clearly defines the rationale.

And if we ever do need either of those, is it not wiser to split them out - for example:

<poem>
lorem
ipsum
</poem>
<hr/>
<poem>
dolor
sit
amet
</poem>

No, it wouldn't.
With appropriate styling, you would then get two blocks of different widths and line across the entire page (or i.e. * * * centered over the entire page) in this case.
The separator needs to accomodate to the widest part of the poem which means it has to be within the poem block.

It seems to me that adding support for <hr/> may solve a problem that maybe shouldn't be used in the first place?

I should be used. The rationale in task description clearly states why. It's a question of semantics and accessibility.

Or am I missing use cases at all?

Yes you are, as described above... ;-)

Huh? Poem does not affect widths, it allows left hand spaces, and allows elimination of <br />, and stuffs up other internal tags.

Of course it does with appropriate styling as I wrote.

If you want to affect widths, you would normally wrap within a centered block or the like. At enWS, the use of Template:*** works fine following a closed poem, and prior to a a new poem tag. Allows total control, and does not suppose anything about hard rules.

Centering of the block does not have any influence on its width.

https://en.wikisource.org/w/index.php?title=Wikisource:Sandbox&diff=prev&oldid=6144443
I am in agreement with @Mooeypoo

I'm sorry, but the fact that you can't imagine something or understand the needs, can't imply that it is not necessary.
https://cs.wikisource.org/w/index.php?title=Wikizdroje:P%C3%ADskovi%C5%A1t%C4%9B&oldid=120147

[offtopic]

I'm sorry, but the fact that you can't imagine something or understand the needs

@Danny_B: For future reference, please criticize ideas instead of people. See the Phabricator etiquette for more information. Thank you for keeping Wikimedia Phabricator a respectful place.

[offtopic]

I'm sorry, but the fact that you can't imagine something or understand the needs

@Danny_B: For future reference, please criticize ideas instead of people. See the Phabricator etiquette for more information. Thank you for keeping Wikimedia Phabricator a respectful place.

Unlike your indirect criticism of myself by your offtopic post, my sentence was not criticizing anybody. It was simple statement that personal views are not arguments.

Change 418420 had a related patch set uploaded (by Gopavasanth; owner: Gopavasanth):
[mediawiki/extensions/Poem@master] No <br /> tag after <hr /> tag to avoid extra space

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

Change 421494 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Poem@master] Cleanup and streamline Poem class

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

Change 421494 abandoned by Thiemo Kreuz (WMDE):
Streamline redundant preg_replace

Reason:
The suggestions I presented here got squashed into I9239da4, as intended.

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

Change 418420 merged by jenkins-bot:
[mediawiki/extensions/Poem@master] No <br /> tag after <hr /> tag to avoid extra space

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

Gopavasanth claimed this task.
Gopavasanth removed a project: Patch-For-Review.