PHPBB3-10620 - Quote tag improvement

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.2/Rhea branch. Everything listed in this forum will be available in phpBB 3.2.
User avatar
AmigoJack
Registered User
Posts: 110
Joined: Wed May 04, 2011 7:47 pm
Location: グリーン ヒル ゾーン
Contact:

Re: PHPBB3-10620 - Quote tag improvement

Post by AmigoJack »

JoshyPHP wrote:

Code: Select all

[quote="admin" post_id="583396" time="1433029874" user_id="2"]
[quote=admin post_id=583396 time=1433029874 user_id=2]
Numbers don't need quotes. But strings do - otherwise I'm the one signing up with the username a post_id=. Make it

Code: Select all

[quote="admin" post_id=583396 time=1433029874 user_id=2]
Also this was already an idea being discussed.
https://www.phpbb.com/community/viewtopic.php?p=13358083#p13358083 wrote:
  1. Post ID should be optional, aswell as username already is.
  2. In the proposed format it will collide with usernames (think of a name like 3, 14) - far better formats would be these:

    Code: Select all

    [quote][/quote]             = No reference
    [quote="name"][/quote]      = Freely chosable name
    [quote="name",1337][/quote] = Freely chosable name and post ID
    [quote=404][/quote]         = Reference to user ID
    [quote=404,1337][/quote]    = Reference to user ID and post ID
  3. I wonder how to distinguish between quoting a post in a PM and quoting a PM in a post.

User avatar
JoshyPHP
Registered User
Posts: 381
Joined: Fri Jul 08, 2011 9:43 pm

Re: PHPBB3-10620 - Quote tag improvement

Post by JoshyPHP »

AmigoJack wrote:Numbers don't need quotes. But strings do [...]
Technically speaking there are very few strings that need quotes in the new parser. Only a string with a ] or a string that contains something that looks like an attribute. I'm considering 3 different strategies for quoting attributes:
  1. Everything in quotes
  2. Only things that are not entirely made of digits
  3. Only things that need to be in quotes, e.g. a username that contains a ]

Code: Select all

[quote="admin" post_id="583396" time="1433029874" user_id="2"]
[quote="admin" post_id=583396 time=1433029874 user_id=2]
[quote=admin post_id=583396 time=1433029874 user_id=2]
As far as the parser is concerned, they're all the same. It makes no difference.

User avatar
Pony99CA
Registered User
Posts: 986
Joined: Sun Feb 08, 2009 2:35 am
Location: Hollister, CA
Contact:

Re: PHPBB3-10620 - Quote tag improvement

Post by Pony99CA »

JoshyPHP wrote:
Pony99CA wrote:Again, if I'm missing something, please give a specific, detailed scenario where information may be leaked.
Create a new topic and preview [quote post_id=123][/quote] as its content. If you see a date and username in the quote then you know that post #123 exists, when it's been posted and who its author is, even if you don't have access to the forum it's in.
I would expect a "You don't have permission to view that forum" type of error if I don't have permission to view the forum that post is in.
JoshyPHP wrote:
Pony99CA wrote:I wasn't debating the meaning of words; I was just pointing out that they have different meanings to different people. You may think that this is simple, but it appears that Cin and I disagree. What's simple for you may not be simple for users.
I'm talking about code complexity, and adding attributes at quoting time is objectively simpler than doing it at posting time.
Now we're going in circles. I didn't dispute whether the coding was simpler; I was saying that only having post ID was simpler from the user's point of view. You argued that things were better for the user if the code was simpler (less chance of bugs), which is true to some degree, but that's why we have QA and Beta testing. Plus, with only a Post ID, you don't have to worry about the other attributes conflicting at all.

You have to keep the old school [quote="user-name"] option for backward compatibility and for people entering quotes manually, of course. (I'm presuming that having three different forms of quotes -- [quote], [quote="user-name"] and [quote=post_id] -- won't be a problem for the new parser.)

Steve
Silicon Valley Pocket PC (http://www.svpocketpc.com)
Creator of manage_bots and spoof_user (ask me)
Need hosting for a small forum with full cPanel & MySQL access? Contact me or PM me.

User avatar
Louis7777
Registered User
Posts: 394
Joined: Fri Apr 04, 2014 12:32 am

Re: PHPBB3-10620 - Quote tag improvement

Post by Louis7777 »

From the user's perspective I think it would be nice and simple if the quote BBCode could only take these forms:
[quote], [quote post_id=123] and [quote="user-name"]

Anything else could be problematic for the user.

Also, maybe the last two could be merged into one. By using double quotation for usernames and have it naked for post numbers (e.g. [quote=123]).

User avatar
AmigoJack
Registered User
Posts: 110
Joined: Wed May 04, 2011 7:47 pm
Location: グリーン ヒル ゾーン
Contact:

Re: PHPBB3-10620 - Quote tag improvement

Post by AmigoJack »

JoshyPHP wrote:As far as the parser is concerned, they're all the same. It makes no difference.
How does it distinguish between attribute="" and attribute= when I want to input "" as string?
JoshyPHP wrote:

Code: Select all

post_id= time= user_id=
Aren't those names longer than they need to be? It's obvious that IDs are used - but inconsistently time= doesn't hint its timestamp nature. Why not post= and user=?
Louis7777 wrote:Also, maybe the last two could be merged into one. By using double quotation for usernames and have it naked for post numbers (e.g. [quote=123]).
As I flagged multiple times: how to distinguish between PMs and posts? Both can use quotation and refer to an ID.

User avatar
JoshyPHP
Registered User
Posts: 381
Joined: Fri Jul 08, 2011 9:43 pm

Re: PHPBB3-10620 - Quote tag improvement

Post by JoshyPHP »

Attribute values are always strings. There's literally no difference between quoted and unquoted.
AmigoJack wrote:Aren't those names longer than they need to be? It's obvious that IDs are used - but inconsistently time= doesn't hint its timestamp nature. Why not post= and user=?
Originally it was post_time, before I realized it would be used for private messages too. So it got shortened to time. It could be named timestamp, I have no strong feelings either way. I named the other attributes after the column names of the values they contain, where applicable.

There's no message_id when quoting private messages. It doesn't seem like something people would use or need.

User avatar
AmigoJack
Registered User
Posts: 110
Joined: Wed May 04, 2011 7:47 pm
Location: グリーン ヒル ゾーン
Contact:

Re: PHPBB3-10620 - Quote tag improvement

Post by AmigoJack »

JoshyPHP wrote:There's literally no difference between quoted and unquoted.
My question was how then a string as "" (opposed to an empty string) can be detected, when it can mean both things: a quoted empty string, and an unquoted string with two quotation marks.
JoshyPHP wrote:There's no message_id when quoting private messages. It doesn't seem like something people would use or need.
From my experience almost all people click "Quote" instead of "Reply", hence all the text is quoted. Which ends up in TOFU orgies. Anyway: PMs can be linked aswell - for every participant of a PM the same URI will be valid.

User avatar
JoshyPHP
Registered User
Posts: 381
Joined: Fri Jul 08, 2011 9:43 pm

Re: PHPBB3-10620 - Quote tag improvement

Post by JoshyPHP »

AmigoJack wrote:My question was how then a string as "" (opposed to an empty string) can be detected, when it can mean both things: a quoted empty string, and an unquoted string with two quotation marks.
The markup is unambiguous, x="" means that the attribute x has an empty string as value.

Double quotes are not allowed in usernames so it's entirely hypothetical, but if someone was named "" then the markup generated for the quote would use single-quotes, e.g. [quote='""']...[/quote]
AmigoJack wrote:PMs can be linked aswell - for every participant of a PM the same URI will be valid.
I'll have to leave that for a future PR.

Nicofuma
3.2 Release Manager
3.2 Release Manager
Posts: 299
Joined: Sun Apr 13, 2014 1:40 am
Location: Paris

Re: PHPBB3-10620 - Quote tag improvement

Post by Nicofuma »

Merged and deployed as it on area51.

Test it intensively and feel free to create a ticket or open a discussion if there is something you don't like.
Member of the phpBB Development-Team
No Support via PM

User avatar
AmigoJack
Registered User
Posts: 110
Joined: Wed May 04, 2011 7:47 pm
Location: グリーン ヒル ゾーン
Contact:

Re: PHPBB3-10620 - Quote tag improvement

Post by AmigoJack »

  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:
    Nicofuma wrote: Tue Jul 07, 2015 7:49 amMerged
  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:
    Nicofuma wrote: Tue Jul 07, 2015 7:49 amMerged
  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:
    AmigoJack wrote: Tue Jul 07, 2015 7:49 amMerged
  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:
    wrote: Tue Jul 07, 2015 7:49 amMerged
  5. Username emptiness handling:
    1. Code: Select all

      [quote="" post_id=281511 time=1436255398 user_id=111569]Merged[/quote]
      wrote: Tue Jul 07, 2015 7:49 amMerged
    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]
      '' wrote: Tue Jul 07, 2015 7:49 amMerged
    4. Code: Select all

      [quote='""' post_id=281511 time=1436255398 user_id=111569]Merged[/quote]
      "" wrote: Tue Jul 07, 2015 7:49 amMerged
    5. Code: Select all

      [quote='' post_id=281511 time=1436255398 user_id=111569]Merged[/quote]
      wrote: Tue Jul 07, 2015 7:49 amMerged
    6. Code: Select all

      [quote='0' post_id=281511 time=1436255398 user_id=111569]Merged[/quote]
      0 wrote: Tue Jul 07, 2015 7:49 amMerged
  6. Parameter commutativity:

    Code: Select all

    [quote=Nicofuma user_id=111569 time=1436255398 post_id=281511]Merged[/quote]
    works great:
    Nicofuma wrote: Tue Jul 07, 2015 7:49 amMerged
  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:
    Nicofuma wrote: Tue Jul 07, 2015 7:49 amMerged
  8. Overflows, out of ranges:
    1. Code: Select all

      [quote=][ post_id=-1 time=-1 user_id=-1]Merged[/quote]
      wrote:[ post_id=-1 time=-1 user_id=-1]Merged
    2. Code: Select all

      [quote='][' post_id=-1 time=-1 user_id=-1]Merged[/quote]
      ][ wrote:Merged
    3. Code: Select all

      [quote=[] post_id=100000000 time=100000000000 user_id=100000000]Merged[/quote]
      [ wrote: post_id=100000000 time=100000000000 user_id=100000000]Merged
    4. Code: Select all

      [quote="[]" post_id=100000000 time=100000000000 user_id=100000000]Merged[/quote]
      [] wrote: Wed Nov 16, 5138 9:46 amMerged
  9. Interfering with make_clickable():

    Code: Select all

    [quote=" http://phpbb.com " post_id=281511 time=1436255398 user_id=111569]Merged[/quote]
    http://phpbb.com wrote: Tue Jul 07, 2015 7:49 amMerged
  10. Using different whitespaces:
    1. U+0009 = Tab:

      Code: Select all

      [quote=Nicofuma	post_id=281511	time=1436255398	user_id=111569]Merged[/quote]
      Nicofuma wrote: Tue Jul 07, 2015 7:49 amMerged
    2. U+000C = Form feed:

      Code: Select all

      [quote=Nicofumapost_id=281511time=1436255398user_id=111569]Merged[/quote]
      Nicofumapost_id=281511time=1436255398user_id=111569 wrote:Merged
    3. U+000A = Line feed:

      Code: Select all

      [quote=Nicofuma
      post_id=281511
      time=1436255398
      user_id=111569]Merged[/quote]
      Nicofuma wrote: Tue Jul 07, 2015 7:49 amMerged
    4. U+000D = Carriage return:

      Code: Select all

      [quote=Nicofuma
      post_id=281511
      time=1436255398
      user_id=111569]Merged[/quote]
      Nicofuma wrote: Tue Jul 07, 2015 7:49 amMerged
    5. U+00A0 No-break space:

      Code: Select all

      [quote=Nicofuma post_id=281511 time=1436255398 user_id=111569]Merged[/quote]
      Nicofuma post_id=281511 time=1436255398 user_id=111569 wrote:Merged
    6. U+2007 = Figure space:

      Code: Select all

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

Post Reply