Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-12501

Weird post attachment behavior in MCP report details

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.1.0-b3
    • Fix Version/s: 3.1.0-b4
    • Component/s: BBCode Engine, Styles
    • Labels:
      None

      Description

      When a post has attachments and is reported, the report details view in the MCP has very strange behavior when it comes to attachments.

      1) When the post has both inline and normal attachments, the order tends to get switched around. So it's possible that the inline attachment is moved to the bottom (normal), and the normal attachment is suddenly inline.

      2) Inline attachments (particularly images) tend to get whole load of extra <br> elements inside of the <div class="inline-attachment">.

        Issue Links

          Activity

          Hide
          PayBas PayBas added a comment - - edited

          I have pinpointed problem 2 to: bbcode_nl2br, but I'm not sure where all the "\n" that bbcode_nl2br is replacing are coming from.

          Show
          PayBas PayBas added a comment - - edited I have pinpointed problem 2 to: bbcode_nl2br, but I'm not sure where all the "\n" that bbcode_nl2br is replacing are coming from.
          Hide
          prototech prototech added a comment - - edited

          They're the empty lines in attachment.html.

          Show
          prototech prototech added a comment - - edited They're the empty lines in attachment.html.
          Hide
          prototech prototech added a comment -

          To reproduce the first issue, you need to add or remove attachments after the post has been reported. Since we store the reported post text, the old indices no longer match the new set of attachments. The only way that I see of fixing this is to only select the attachments that were uploaded before the post was reported, which would also follow the new behavior in Ascraeus of preserving the content in its reported state.

          An ideal fix would be to get rid of the indices entirely and use the attachment id to identify the files, but would require a major undertaking as we would need to reparse all posts. This would solve several scenarios where the indices in the attachment bbcode can be corrupted when deleting attachments outside of posting.php.

          Show
          prototech prototech added a comment - To reproduce the first issue, you need to add or remove attachments after the post has been reported. Since we store the reported post text, the old indices no longer match the new set of attachments. The only way that I see of fixing this is to only select the attachments that were uploaded before the post was reported, which would also follow the new behavior in Ascraeus of preserving the content in its reported state. An ideal fix would be to get rid of the indices entirely and use the attachment id to identify the files, but would require a major undertaking as we would need to reparse all posts. This would solve several scenarios where the indices in the attachment bbcode can be corrupted when deleting attachments outside of posting.php.
          Hide
          PayBas PayBas added a comment -

          Ah, this is a more complex problem than I thought.

          Show
          PayBas PayBas added a comment - Ah, this is a more complex problem than I thought.
          Hide
          nickvergessen Joas Schilling added a comment -

          Should be fixed for PM reports aswell

          Show
          nickvergessen Joas Schilling added a comment - Should be fixed for PM reports aswell
          Hide
          prototech prototech added a comment -

          The PM in the report is displayed in its current state, instead of the state in which it was when reported, so PM's are unaffected.

          Show
          prototech prototech added a comment - The PM in the report is displayed in its current state, instead of the state in which it was when reported, so PM's are unaffected.

            People

            • Assignee:
              prototech prototech
              Reporter:
              PayBas PayBas
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development