WordPress.org

Make WordPress Core

Opened 12 months ago

Closed 10 months ago

Last modified 10 months ago

#24159 closed defect (bug) (invalid)

Chats with spaces in the 'speaker' not parsing correctly

Reported by: Ipstenu Owned by:
Milestone: Priority: normal
Severity: major Version: 3.6
Component: Post Formats Keywords: has-patch
Focuses: Cc:

Description

Since #23625 is closed and we were asked to make a new ticket, the issue I mentioned still remains.

Related to #23947 - currently this is broken in instances where chat names have spaces.

http://test.ipstenu.org/testing-1.2013/

That has the post content as follows:

Nigel Tufnel: The numbers all go to eleven. Look, right across the board, eleven, eleven, eleven and…

Marti DiBergi: Oh, I see. And most amps go up to ten?

Nigel Tufnel: Exactly.

Marti DiBergi: Does that mean it’s louder? Is it any louder?

Nigel Tufnel: Well, it’s one louder, isn’t it? It’s not ten. You see, most blokes, you know, will be playing at ten. You’re on ten here, all the way up, all the way up, all the way up, you’re on ten on your guitar. Where can you go from there? Where?

Marti DiBergi: I don’t know.

Nigel Tufnel: Nowhere. Exactly. What we do is, if we need that extra push over the cliff, you know what we do?

Marti DiBergi: Put it up to eleven.

Nigel Tufnel: Eleven. Exactly. One louder.

Marti DiBergi: Why don’t you just make ten louder and make ten be the top number and make that a little louder?

Nigel Tufnel: These go to eleven.

But nothing shows.

You can see the exact same issue here: http://twentythirteendemo.wordpress.com/2013/02/10/never-say-never-say-never/

James Bond's lines are never shown.

Attachments (2)

24159.diff (2.5 KB) - added by belloswan 12 months ago.
24159.2.diff (611 bytes) - added by aaroncampbell 11 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 wonderboymusic12 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 3.6

I will check the Unit Tests and make sure the expected output is correct, then make sure the theme is parsing properly

comment:2 belloswan12 months ago

  • Cc antoine@… added
  • Keywords needs-testing added

This patch fixs this bug but doesn't allow the chat be presented like this :
Username:
[Line-break]
Message
[Line-break]
Username:
[Line-break]
Message

It has to be like this:
Username:
Message
Username:
Message

belloswan12 months ago

comment:3 wonderboymusic12 months ago

Did you run Unit Tests against this? I have a feeling this is going to be no bueno

comment:4 belloswan12 months ago

Yes it does work but the Unit Tests chat post is really simple...

comment:5 wonderboymusic12 months ago

  • Keywords has-patch needs-unit-tests added; needs-patch removed

Cool - could you add some more Unit Tests related to this ticket?

comment:6 lancewillett12 months ago

See also #24240 for a similar case with Quote formats.

comment:7 aaroncampbell11 months ago

Ipstenu: Where did that chat get copied from? The chat system has different parsers for IM and Skype, and I'm wondering which should be handling this.

aaroncampbell11 months ago

comment:8 aaroncampbell11 months ago

First, 24159.2.diff fixes this particular use case. However, it only fixes it because there's an empty line between each stanza. Without that, chats with spaces in the usernames will still break.

The core issue seems to be that what we're trying to handle this scenario:

Scott: Hey.
I have a question: what is your favorite color?
Nacin: Go away.

The problem is that while I see what we're trying to do, in almost every chat program "I have a question" is a valid username. I think we're trying to be too smart here.

Maybe we could check for spaces or tabs only at the beginning of the username? Then we could support these without compromising spaces in usernames:

Scott: Hey.
  I have a question: what is your favorite color?
Nacin: Go away.
Scott: Hey.
	I have a question: what is your favorite color?
Nacin: Go away.

comment:9 ryan11 months ago

I'd be okay with being less smart, and thus more reliable.

Version 0, edited 11 months ago by ryan (next)

comment:10 Ipstenu11 months ago

The sample text came from the TwentyThirteen demo site.

Keep in mind, chats aren't just "I'm copying this straight from IM" but also used today as a way to transcribe interviews and radio events etc. With that in mind, there should be a fall back to handle spaces in names, if Skype etc aren't 'detected'

comment:11 wonderboymusic10 months ago

  • Milestone 3.6 deleted
  • Resolution set to invalid
  • Status changed from new to closed

Chat code was thrown in the trash can (editor's note: ..... )

comment:13 SergeyBiryukov10 months ago

  • Keywords needs-testing needs-unit-tests removed
Note: See TracTickets for help on using tickets.