[Ticket/10845] Reported post text was not parsing BBCode #786

Merged
merged 5 commits into from Nov 6, 2012

Projects

None yet

7 participants

@brunoais

This fixes the BBCode not being parsed by saving the post's bitfield and uid in the database to be retrieved while reading the saved post.
While viewing the report, the preview post is now parsed by the BBCode system and shows exactly like a user would see except still the text is not censored.

brunoais added some commits Apr 26, 2012
@brunoais brunoais [ticket/10845] Changed the report system. Now it saves posts with the…
… bbcode

Now the bitfield and uid of the bbcode is saved in the reports table.
This will allow parsing the BBCode while loading the post to show

PHPBB3-10845
f3e5acf
@brunoais brunoais [ticket/10845] Now it shows the preview post parsed
With this addition, the preview post in the report details now shows with the
BBCode parsed.

Note: Tested with all BBCodes I have including one personalised to write the <s>
BBCode.

PHPBB3-10845
06f4ef8
@nickvergessen nickvergessen and 1 other commented on an outdated diff May 2, 2012
phpBB/includes/mcp/mcp_reports.php
@@ -227,7 +227,7 @@ function main($id, $mode)
'REPORTER_NAME' => get_username_string('username', $report['user_id'], $report['username'], $report['user_colour']),
'U_VIEW_REPORTER_PROFILE' => get_username_string('profile', $report['user_id'], $report['username'], $report['user_colour']),
- 'POST_PREVIEW' => bbcode_nl2br($report['reported_post_text']),
+ 'POST_PREVIEW' => bbcode_nl2br(generate_text_for_display($report['reported_post_text'], $report['reported_post_uid'], $report['reported_post_bitfield'], OPTION_FLAG_BBCODE + OPTION_FLAG_SMILIES + OPTION_FLAG_LINKS)),
@nickvergessen
nickvergessen May 2, 2012

doesnt generate_text_for_display() do the bbcode_nl2br() ?

@brunoais
brunoais May 2, 2012

Please use a complete sentence I didn't understand (explained that generate_text_for_display() already applies bbcode_nl2br in IRC).

Fixed

@brunoais brunoais [ticket/10845] Remove censor from text
In order to remove the censor, I added a new parameter to
generate_text_for_display(), that new parameter is used to tell if the text
should be censored or not. Defaults to true.

PHPBB3-10845
593ef78
@brunoais

Ups. There's one thing missing here.
I forgot to register the changes to the DB in the DB update files. Will do in some minutes

@brunoais brunoais [ticket/10845] Two of the three colouns were missing in the DB update…
… file

Added the two colouns that were missing in the DB update file

PHPBB3-10845
6069d9a
@travisbot

This pull request fails (merged 6069d9a into beda22c).

@callumacrae callumacrae commented on the diff May 16, 2012
phpBB/develop/create_schema_files.php
- 'user_notify' => array('BOOL', 0),
- 'report_closed' => array('BOOL', 0),
- 'report_time' => array('TIMESTAMP', 0),
- 'report_text' => array('MTEXT_UNI', ''),
- 'reported_post_text' => array('MTEXT_UNI', ''),
+ 'report_id' => array('UINT', NULL, 'auto_increment'),
+ 'reason_id' => array('USINT', 0),
+ 'post_id' => array('UINT', 0),
+ 'pm_id' => array('UINT', 0),
+ 'user_id' => array('UINT', 0),
+ 'user_notify' => array('BOOL', 0),
+ 'report_closed' => array('BOOL', 0),
+ 'report_time' => array('TIMESTAMP', 0),
+ 'report_text' => array('MTEXT_UNI', ''),
+ 'reported_post_text' => array('MTEXT_UNI', ''),
+ 'reported_post_uid' => array('VCHAR:8', ''),
@callumacrae
callumacrae May 16, 2012

Not sure what's happening here, but there should only be three additions.

@brunoais
brunoais May 16, 2012

Count the number of whitespace characters.
In order to have the => aligned, all of them require a new tab.

@imkingdavid imkingdavid commented on the diff Jun 1, 2012
phpBB/includes/mcp/mcp_reports.php
@@ -227,7 +227,7 @@ function main($id, $mode)
'REPORTER_NAME' => get_username_string('username', $report['user_id'], $report['username'], $report['user_colour']),
'U_VIEW_REPORTER_PROFILE' => get_username_string('profile', $report['user_id'], $report['username'], $report['user_colour']),
- 'POST_PREVIEW' => bbcode_nl2br($report['reported_post_text']),
+ 'POST_PREVIEW' => generate_text_for_display($report['reported_post_text'], $report['reported_post_uid'], $report['reported_post_bitfield'], OPTION_FLAG_BBCODE | OPTION_FLAG_SMILIES, false),
@imkingdavid
imkingdavid Jun 1, 2012

Why are only BBCode and Smilies allowed, shouldn't it also parse magic urls?

@brunoais
brunoais Jun 2, 2012

I don't know. I'm ok with both (the current and with magic urls).

@imkingdavid
imkingdavid Jun 2, 2012

IMO, all three should be parsed. Also, aren't you supposed to add the flags together when passing them to generate_text_for_display, rather than using bitwise OR? (i.e. OPTION_FLAG_BBCODE + OPTION_FLAG_SMILIES)

@brunoais
brunoais Jun 3, 2012

That: OPTION_FLAG_BBCODE + OPTION_FLAG_SMILIES
Is only equivalent to:
OPTION_FLAG_BBCODE | OPTION_FLAG_SMILIES
because there don't have not shared bits.
I.e.
OPTION_FLAG_BBCODE = 001
OPTION_FLAG_SMILIES = 010
OPTION_FLAG_LINKS = 100

The bitwise OR is the standard(?) (at least the most logical to me and what's teached). I don't know about the specifics and which one is faster but the bitwise OR is what makes more sense to me. If there's something in the coding guidelines that I missed, I'll fix it.

@imkingdavid
imkingdavid Jun 3, 2012
@brunoais
brunoais Jun 3, 2012
@nickvergessen
nickvergessen Aug 20, 2012

In posts table we have 3 columns for this (enable_bbcodes, *_smilies, *_urls) which should be copied and used here aswell, otherwise the post is still not the same as it is in the viewtopic view.

However I guess you can combine them to one "reported_post_text_options" field

@imkingdavid imkingdavid and 1 other commented on an outdated diff Jun 14, 2012
phpBB/report.php
@@ -150,16 +154,19 @@
trigger_error('EMPTY_REPORT');
}
+
@imkingdavid
imkingdavid Jun 14, 2012

There should not be a new line added here.

@brunoais
brunoais Jun 14, 2012

Hum... You're right... I wonder why that got there...

@imkingdavid

Other than the comment on the newline, I would like someone else to provide input on the question I had about whether or not to parse magic url (IMO it should be parsed), and then this should be ready for merge.

@brunoais brunoais [ticket/10845] Removed one empty line that wasn't supposed to be there
I onder when it got there... Maybe a wrong rebase... Don't really know.

PHPBB3-10845
27775e8
@travisbot

This pull request passes (merged 27775e8 into beda22c).

@brunoais

I'm still waiting for feedback here...
Feedback please!

@Senky

Yes, please, include magic urls!

Another idea comes to my mind: if DB is being updated, 2 new columns are added to REPORTS_TABLE, but they will stay empty for all already existing reports, do not they? Do not we need to fill them while updating database?

@nickvergessen
phpBB Forum Software member

I think updating all of them is not needed. That might be a rather huge task for little purpose

@nickvergessen
phpBB Forum Software member

Regarding magic urls, they should be parsed, if they are parsed in the actual post.

@p
p commented Nov 6, 2012

http://tracker.phpbb.com/browse/PHPBB3-11170, I only now realized this is a bug.

@p p added a commit to p/phpbb3 that referenced this pull request Nov 6, 2012
@p p Merge PR #786 branch 'brunoais/ticket/10845' into develop
* brunoais/ticket/10845:
  [ticket/10845] Removed one empty line that wasn't supposed to be there
  [ticket/10845] Two of the three colouns were missing in the DB update file
  [ticket/10845] Remove censor from text
  [ticket/10845] Now it shows the preview post parsed
  [ticket/10845] Changed the report system. Now it saves posts with the bbcode

Conflicts:
	phpBB/includes/functions_content.php
196ed8e
@p p merged commit 27775e8 into phpbb:develop Nov 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment