search

flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.3k stars 835 forks source link

Post content be wrapped before validation #1090

Closed oanhnn closed 7 years ago

oanhnn commented 7 years ago

Bug report

I want check post content maximum is 1000 characters, but post content will be wrapped by <t> and </t> before it is validated. when we using flarum-ext-markdown extension, content will be wrapped by <t><p> and </p></t>. We can confirm it and see it in database. Is it issue of flarum?

Flarum info

Flarum core 0.1.0-beta.6
PHP 7.0.8-0ubuntu0.16.04.3
Loaded extensions: Core, date, libxml, openssl, pcre, zlib, filter, hash, pcntl, Reflection, SPL, session, standard, mysqlnd, PDO, xml, calendar, ctype, curl, dom, mbstring, fileinfo, ftp, gd, gettext, iconv, json, exif, mcrypt, mysqli, pdo_mysql, Phar, posix, readline, shmop, SimpleXML, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, wddx, xmlreader, xmlwriter, xsl, zip, Zend OPcache
EXT s9e-autoimage 1.0.2 (579c8f47d6d654f38a1965b1b2fd9241d3f096a5)
EXT s9e-autovideo 1.0.0 (73b05accb37e7b6cba3c0f46c4bb0a7c5ecea147)
EXT flarum-bbcode v0.1.0-beta.5
EXT wiwatsrt-best-answer v0.1.0-beta.5
EXT hyn-default-group dev-master (7dc4d8c8281c0b0e0f693f27fc18df8bf8ea1edd)
EXT sijad-details dev-master (9efd34bd8291a3b77a055a88bf0896444601caa6)
EXT flarum-emoji v0.1.0-beta.6
EXT flarum-english v0.1.0-beta.6
EXT flagrow-upload dev-master
EXT flarum-flags v0.1.0-beta.6
EXT flarum-likes v0.1.0-beta.6
EXT sijad-links dev-master (10f25c478373f78f33267f3be462dda725b414c8)
EXT flarum-lock v0.1.0-beta.6
EXT flarum-markdown v0.1.0-beta.5
EXT s9e-mediaembed 0.3.2 (2d4fce9da26cdff381c101f9c8dc4aded8bfee5d)
EXT sijad-pages dev-master
EXT sijad-spoiler-alert dev-master (d238f433f327b9d256429b1965995966b30a4108)
EXT flarum-sticky v0.1.0-beta.6
EXT flarum-suspend v0.1.0-beta.6
EXT flarum-tags v0.1.0-beta.7
EXT avatar4eg-transliterator dev-master (9a2ebf949e406f4cc1449d6b3597bf8536721e9f)
EXT avatar4eg-users-list dev-master (ed9b1a02832fb00031ab1c0030b18071751464c4)
Base URL: http://127.0.0.1:8080
Installation path: /app

Additional comments

Current, i must set maximum 1014 characters

luceos commented 7 years ago

To be honest I don't see why this is a bug. You will never know what extensions modify the content of a post. Your 1000 character limit therefore will always be an approximation. Best you can do if you want to make your approximation as best as possible is to take into consideration the mutations you've already noticed from widely used extensions.

I've considered the option of transforming the generated html back to raw text, but in that case you would always lose all markup. Another option is parsing all paragraphs or parts of the content and then applying your limits on those, from what I understand that might be the best way to move forward.

oanhnn commented 7 years ago

@Luceos What are you think about if we convert content to string and validate length of it? I think it is expectation of all users

luceos commented 7 years ago

@oanhnn I was updating my comment as you replied. The last option might be the best thing to do. It will require some kind of html parsing to be introduced, but packages for these abound.

oanhnn commented 7 years ago

I found http://php.net/manual/en/function.strip-tags.php It can help we fix this problem. :+1:

luceos commented 7 years ago

If you only do validation, that is possible yes.

This issue can be closed.

oanhnn commented 7 years ago

This issue can be closed.

I think this issue still open until any one make PR to resolved it.

luceos commented 7 years ago

I disagree, there is nothing wrong with with what Flarum does with content now. Your extension has to handle that logic by itself, not Flarum.