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

Section edit changes produce invalid HTML in Wrap plugin #2304

Closed
selfthinker opened this issue Apr 8, 2018 · 6 comments
Closed

Section edit changes produce invalid HTML in Wrap plugin #2304

selfthinker opened this issue Apr 8, 2018 · 6 comments
Labels
Milestone

Comments

@selfthinker
Copy link
Collaborator

Since the section edit changes (#2220), the Wrap plugin now produces invalid HTML. When using the syntax example page there are 2 unclosed divs and another div is missing.

I'm not 100% sure if the method in question is just used wrongly by the plugin or if it's an issue in the core. Although after playing around a bit, I suspect it's in the core. And I am 100% sure it was caused by that change as it is fine in the parent commit.
See selfthinker/dokuwiki_plugin_wrap#163 for more info.

@lpaulsen93
Copy link
Collaborator

I will check it a.s.a.p..

@lpaulsen93
Copy link
Collaborator

I think in inc/html.php the code section

if (!defined('SEC_EDIT_PATTERN')) {
    define('SEC_EDIT_PATTERN', '#<!-- EDIT({.*?}) -->#');
}

has to be replaced with

if (!defined('SEC_EDIT_PATTERN')) {
    define('SEC_EDIT_PATTERN', '#<!-- EDIT({.*?}) -->#');
}

to make the pattern not greedy. Fixed at least unclosed elements with this wrap code:

<WRAP group>
<WRAP third column>
=== Widthsfdgh1 ===

You can set any valid widths (but only on divs): ''%, px, em, rem, ex, ch, vw, vh, pt, pc, cm, mm, in'', but most of the time you'd only want either

^type^e.g.^note^
^''%''|''30%''|makes sense in most cases|
^''px''|''420px''|makes sense if your container contains images with a certain width|
^''em''|''20em''|makes sense if you like your wrap container to grow and shrink with the font size|

</WRAP>
</WRAP>

Nested sections were not working due to the greedy pattern. Haven't got more time today. Will do more testing with the complete wrap example page in the next days.

I did not notice it before but in the bad case the edit button for the table (from plugin edittable) was missing because it was nested in a wrap section.

@splitbrain splitbrain added the Bug label Apr 9, 2018
@splitbrain splitbrain added this to the 🐱 Greebo milestone Apr 9, 2018
@Klap-in
Copy link
Collaborator

Klap-in commented Apr 10, 2018

btw, to reduce eventually confusion, the first code example has a typo:
https://github.com/splitbrain/dokuwiki/blob/ec57f1190eb6df5ddeba76bb2c35b46d65d31561/inc/html.php#L11-L13

@lpaulsen93
Copy link
Collaborator

@Klap-in: aaargh, yes copy-and-paste. Thanks 👍 (I guess I was tired)

@lpaulsen93
Copy link
Collaborator

@selfthinker: should be fixed now.

@splitbrain: sorry, accidentally pushed to master directly. That wasn't my intention - forgot git checkout issue2304.

@selfthinker
Copy link
Collaborator Author

Thanks @LarsGit223, I can confirm everything is valid again.

@splitbrain, GitHub has the option to protect branches, i.e. the master branch could be protected from pushing to without a PR (that can include or exclude administrators). I think it's generally worth having.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants