Make WordPress Core

Opened 15 years ago

Closed 10 years ago

Last modified 10 years ago

#8775 closed defect (bug) (fixed)

Numbers in quotation marks get wrong smart quotes

Reported by: filosofo's profile filosofo Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.0 Priority: normal
Severity: normal Version: 2.8
Component: Formatting Keywords: has-patch needs-testing wptexturize
Focuses: Cc:

Description

Have a number in quotation marks, such as "12345" or '12345' and wptexturize converts the right quotation mark to double-prime and prime marks, respectively.

Patch fixes.

Attachments (7)

number_curly_quotes.diff (1.3 KB) - added by filosofo 15 years ago.
number_curly_quotes.2.diff (1.3 KB) - added by mrmist 15 years ago.
Update against current trunk
8775-quotes-patch.diff (1.2 KB) - added by aliso 12 years ago.
Quotation mark fix for most recent code in formatting.php
before-8775-patch.png (1.5 KB) - added by MikeHansenMe 12 years ago.
screenshot of problem
after-8775-patch.png (2.1 KB) - added by MikeHansenMe 12 years ago.
screenshot after patch
8775.4.diff (1.2 KB) - added by MikeHansenMe 12 years ago.
Updated patch path relative from wp folder
miqro-8775.patch (3.7 KB) - added by miqrogroove 10 years ago.

Download all attachments as: .zip

Change History (49)

#1 @mrmist
15 years ago

Probably a dupe see also #7754 #4539 #3810 #4116 #1258

There neeeds to be a big old wp_texturize fix task or something.

#2 @ryan
15 years ago

  • Component changed from General to Formatting
  • Owner anonymous deleted

#4 @Denis-de-Bernardy
15 years ago

  • Keywords needs-testing added

@mrmist
15 years ago

Update against current trunk

#5 @mrmist
15 years ago

  • Keywords tested commit added; needs-testing removed

Updated patch. Tested with numbers and single/double quotes. Seems to work. doesn't appear to break anything else, so marking as tested. Comitting would seem the fastest way of geting more test exposure.

#7 follow-up: @Denis-de-Bernardy
15 years ago

are there any unit tests on wptexturize?

#8 @Denis-de-Bernardy
15 years ago

or at least, known, funky test cases to watch for?

#9 @azaozz
15 years ago

  • Keywords needs-testing added; tested wptexturize formatting curly-quotes numbers commit removed

#10 in reply to: ↑ 7 @azaozz
15 years ago

Replying to Denis-de-Bernardy:

are there any unit tests on wptexturize?

Seems like this needs more testing with non-latin languages.

#11 @Denis-de-Bernardy
15 years ago

  • Component changed from Formatting to i18n
  • Owner set to nbachiyski

k... moving this over to i18n then. I wouldn't know what to test on this front. :-)

D.

#12 @azaozz
15 years ago

Also see #3810.

#13 @Denis-de-Bernardy
15 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from 2.8 to Future Release

punting pending unit tests... even though, like the one wpautop one I just punted, I think this should get committed outright.

#14 @Denis-de-Bernardy
15 years ago

  • Component changed from i18n to Formatting
  • Milestone changed from Future Release to 2.9
  • Owner nbachiyski deleted

#15 @janeforshort
14 years ago

  • Milestone changed from 2.9 to Future Release

Punting b/c no tests done in 6 months and we're in beta.

#16 follow-up: @filosofo
14 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Let's be realistic.

#17 in reply to: ↑ 16 ; follow-up: @Viper007Bond
13 years ago

  • Milestone set to Future Release
  • Resolution wontfix deleted
  • Status changed from closed to reopened

It's still a valid issue. Just because progress isn't being made isn't reason enough to close the ticket.

#18 in reply to: ↑ 17 @Denis-de-Bernardy
13 years ago

Replying to Viper007Bond:

It's still a valid issue. Just because progress isn't being made isn't reason enough to close the ticket.

Any chances you've got a patch? :-)

#19 @filosofo
13 years ago

We have a patch. It's the prerequisite wptexturize unit tests that are missing.

@aliso
12 years ago

Quotation mark fix for most recent code in formatting.php

#21 @aliso
12 years ago

It looks like the previous patch won't work with the code change from [19795]. I attached a patch that fixes it in the most recent code.

This also solves part 1 of #16957 (same bug).

@MikeHansenMe
12 years ago

screenshot of problem

@MikeHansenMe
12 years ago

screenshot after patch

@MikeHansenMe
12 years ago

Updated patch path relative from wp folder

#22 @MikeHansenMe
12 years ago

  • Cc mdhansen@… added

aliso's patch works for me. I updated the patch path and included some screenshots of the problem.

Last edited 12 years ago by MikeHansenMe (previous) (diff)

#23 @damst
11 years ago

  • Cc damst added

#25 @aliso
11 years ago

Any idea when this fix can make it into core? The unit tests should be the same as in http://unit-tests.trac.wordpress.org/changeset/757, right?

#26 @nacin
10 years ago

  • Keywords wptexturize added

#27 @miqrogroove
10 years ago

What is this intended output for these examples?

I need 4 x 20'=80' of trim.

The best year "was that time in 2012" when everyone partied, he said.

#28 @miqrogroove
10 years ago

I recommend narrowing the focus of this patch so that it will not affect text other than numbers surrounded by quotes preceded by spaces. Examples:

"2004"
'27'
The answer is "42".

Numbers at the beginning or ending of quoted phrases have ambiguous meaning if we want to keep using the primes logic.

#29 @miqrogroove
10 years ago

#14491 was marked as a duplicate.

#30 @miqrogroove
10 years ago

#16957 was marked as a duplicate.

#31 @miqrogroove
10 years ago

  • Keywords needs-unit-tests removed

New patch against trunk should be much closer to what we need. In miqro-8775.patch:

  • Quoted numbers preceded by spaces get curly quotes.
  • So, "123" works, but"123"blah does not work.
  • Trailing space not required. Will this break things like '99's?
  • Decimals and commas are allowed after the first digit.
  • nbsp works as whitespace.
  • Regexp will search for quotes first for optimum performance.
  • Unit tests updated to match.

#32 @miqrogroove
10 years ago

This will also break the assertion for 'Class of '99', but that is highly ambiguous.

#33 @wonderboymusic
10 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from reopened to closed

In 28721:

Fix curly quotes around numbers when applicable.

Adds unit tests.

Props filosofo, mrmist, aliso, MikeHansenMe, miqrogroove.
Fixes #8775.

#34 @miqrogroove
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen to discuss and update the broken unit test for

Class of '99's

See also #26850.

The main question is which patterns should have priority when an apostrophe preceeds exactly two digits.

#35 @miqrogroove
10 years ago

This is fixed now.

#36 @samuelsidler
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

#37 @TobiasBg
10 years ago

  • Milestone changed from Future Release to 4.0

#39 @helen
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for investigation.

#40 @miqrogroove
10 years ago

coreygilmore: You are seeing the prime and double-prime logic there. It hasn't changed.

#41 @helen
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Apparently this was for numbers-only within quotes.

#42 @miqrogroove
10 years ago

Same situation as https://core.trac.wordpress.org/ticket/4539#comment:57

Several different bugs were brought up in comments but never really had a whole ticket just for "Primes vs. Quotes Ending With Digits". That is its own can of worms.

Make a new ticket if desired.

Note: See TracTickets for help on using tickets.