PHPBB3-10620 - Quote tag improvement

[JoshyPHP]

Merged and deployed as it on area51.

Would you mind purging the template cache? Quoting from the topic review doesn't have the new attributes.

Interfering with make_clickable():

Quick correction: make_clickable() isn't used anymore. It's all in the BBCode definition. If the name used for the quote starts with "http://" it's used as a URL if there was none specified.

Using different whitespaces:

I've just added support in the library for more whitespace between attributes to allow newlines and tabs. That seems to make sense.

[Nicofuma]

done

[brunoais]

Re-doing AmigoJack's post with extra tests:

1. Clicking the quote button creates the BBCode as intended:

   Code: Select all

   [quote=Nicofuma post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

   renders as:

   Merged

2. Robustness when skipping user ID in BBCode:

   Code: Select all

   [quote=Nicofuma post_id=281511 time=1436255398]Merged[/quote]

   as expected: username is not clickable anymore:

   Merged

3. Of course, this can now be used to spoof usernames which then link to other profiles:

   Code: Select all

   [quote=AmigoJack post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

   renders as:

   Merged

4. Trying to leave username empty:

   Code: Select all

   [quote= post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

   renders naively an empty A element (<a href="./memberlist.php?mode=viewprofile&u=111569"></a>), which can never be clicked:

   Merged

5. Username emptiness handling:

   1. Code: Select all

      [quote="" post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

      Merged

   2. Code: Select all

      [quote="""" post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

      [quote="""" post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

   3. Code: Select all

      [quote="''" post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

      Merged

   4. Code: Select all

      [quote='""' post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

      Merged

   5. Code: Select all

      [quote='' post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

      Merged

   6. Code: Select all

      [quote='0' post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

      Merged

6. Parameter commutativity:

   Code: Select all

   [quote=Nicofuma user_id=111569 time=1436255398 post_id=281511]Merged[/quote]

   works great:

   Merged

7. Parameter domination on duplicate attributes:

   Code: Select all

   [quote=Nicofuma post_id=281511 post_id=281166 time=1400000000 time=1436255398 user_id=111569 user_id=1 user_id=90321]Merged[/quote]

   Last one wins:

   Merged

8. Overflows, out of ranges:

   1. Code: Select all

      [quote=][ post_id=-1 time=-1 user_id=-1]Merged[/quote]

      [ post_id=-1 time=-1 user_id=-1]Merged

   2. Code: Select all

      [quote='][' post_id=-1 time=-1 user_id=-1]Merged[/quote]

      Merged

   3. Code: Select all

      [quote=[] post_id=100000000 time=100000000000 user_id=100000000]Merged[/quote]

      post_id=100000000 time=100000000000 user_id=100000000]Merged

   4. Code: Select all

      [quote="[]" post_id=100000000 time=100000000000 user_id=100000000]Merged[/quote]

      Merged

9. Interfering with make_clickable():

   Code: Select all

   [quote=" http://phpbb.com " post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

   Merged

10. Using different whitespaces, no quotes:
    1. U+0009 = Tab:

       Code: Select all

       [quote=Nicofuma	post_id=281511	time=1436255398	user_id=111569]Merged[/quote]

       Merged

    2. U+000C = Form feed:

       Code: Select all

       [quote=Nicofumapost_id=281511time=1436255398user_id=111569]Merged[/quote]

       Merged

    3. U+000A = Line feed:

       Code: Select all

       [quote=Nicofuma
       post_id=281511
       time=1436255398
       user_id=111569]Merged[/quote]

       Merged

    4. U+000D = Carriage return:

       Code: Select all

       [quote=Nicofuma
       post_id=281511
       time=1436255398
       user_id=111569]Merged[/quote]

       Merged

    5. U+00A0 No-break space:

       Code: Select all

       [quote=Nicofuma post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

       Merged

    6. U+2007 = Figure space:

       Code: Select all

       [quote=Nicofuma post_id=281511 time=1436255398 user_id=111569]Merged[/quote]

       Merged

11. Using different whitespaces with quotes:
    1. U+0009 = Tab:

       Code: Select all

       [quote="Nicofuma"	post_id="281511"	time="1436255398"	user_id="111569"]Merged[/quote]

       Merged

    2. U+000C = Form feed:

       Code: Select all

       [quote="Nicofuma"post_id="281511"time="1436255398"user_id="111569"]Merged[/quote]

       Merged

    3. U+000A = Line feed:

       Code: Select all

       [quote="Nicofuma"
       post_id="281511"
       time="1436255398"
       user_id="111569"]Merged[/quote]

       Merged

    4. U+000D = Carriage return:

       Code: Select all

       [quote="Nicofuma"
       post_id="281511"
       time="1436255398"
       user_id="111569"]Merged[/quote]

       [quote="Nicofuma"
       post_id="281511"
       time="1436255398"
       user_id="111569]Merged[/quote]
    5. U+00A0 No-break space:

       Code: Select all

       [quote="Nicofuma" post_id="281511" time="1436255398" user_id="111569"]Merged[/quote]

       Merged

    6. U+2007 = Figure space:

       Code: Select all

       [quote="Nicofuma" post_id="281511" time="1436255398" user_id="111569"]Merged[/quote]

       [quote="Nicofuma" post_id="281511" time="1436255398" user_id="111569"]Merged[/quote]

[Kailey]

I'm late to the party Just a thought, is it possible to grab the color of the quoted author also?

[DavidIQ]

I'm late to the party Just a thought, is it possible to grab the color of the quoted author also?

Colors can change. Not sure that is such a good idea to forever immortalize a user's current color/team designation in a quote, although it's not that big of a deal.

BTW I've noticed that quoting someone now adds two new lines after the closing quote tag. Is this on purpose? Also has anyone checked what happens when you quote an anonymous user or when the quoted user no longer exists (account deleted)?

[JoshyPHP]

BTW I've noticed that quoting someone now adds two new lines after the closing quote tag. Is this on purpose?

It is, as per https://tracker.phpbb.com/browse/PHPBB3-13901

Also has anyone checked what happens when you quote an anonymous user or when the quoted user no longer exists (account deleted)?

Anonymous user: it uses the guest name like in phpBB 3.1, and it links to an inexistent profile. The latter part is fixed by https://github.com/phpbb/phpbb/pull/3758

I haven't checked the other case but I think that posts are reassigned to the anonymous user when a user is deleted.

[AmigoJack]

posts are reassigned to the anonymous user when a user is deleted

Partially: per post poster_id is set to ANONYMOUS, but post_username is filled to still distinguish between multiple deleted accounts. The username can live on without an account.

[Kailey]

Colors can change.

I wasn't implying storing it in the posts table. We already do calls to the users table (and if I remember correctly, the entire users table) for viewtopic. Would it not be possible to match the quoted user_id to the appropriate user_colour? Just food for thought.

[DavidIQ]

I wasn't implying storing it in the posts table. We already do calls to the users table (and if I remember correctly, the entire users table) for viewtopic. Would it not be possible to match the quoted user_id to the appropriate user_colour? Just food for thought.

Ah ok. That makes more sense. But what would/should happen when the quote is not from the current topic?

[AmigoJack]

not from the current topic

Or not from the current page? What about search results? And signatures? And forum descriptions? Yes, quotes are not really expected, but not forbidden either.