WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 13 months ago

Last modified 13 months ago

#8775 closed defect (bug) (fixed)

Numbers in quotation marks get wrong smart quotes

Reported by: filosofo Owned by: 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 7 years ago.
number_curly_quotes.2.diff (1.3 KB) - added by mrmist 6 years ago.
Update against current trunk
8775-quotes-patch.diff (1.2 KB) - added by aliso 3 years ago.
Quotation mark fix for most recent code in formatting.php
before-8775-patch.png (1.5 KB) - added by MikeHansenMe 3 years ago.
screenshot of problem
after-8775-patch.png (2.1 KB) - added by MikeHansenMe 3 years ago.
screenshot after patch
8775.4.diff (1.2 KB) - added by MikeHansenMe 3 years ago.
Updated patch path relative from wp folder
miqro-8775.patch (3.7 KB) - added by miqrogroove 17 months ago.

Download all attachments as: .zip

Change History (49)

comment:1 @mrmist7 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.

comment:2 @ryan6 years ago

  • Component changed from General to Formatting
  • Owner anonymous deleted

comment:4 @Denis-de-Bernardy6 years ago

  • Keywords needs-testing added

@mrmist6 years ago

Update against current trunk

comment:5 @mrmist6 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.

comment:7 follow-up: @Denis-de-Bernardy6 years ago

are there any unit tests on wptexturize?

comment:8 @Denis-de-Bernardy6 years ago

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

comment:9 @azaozz6 years ago

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

comment:10 in reply to: ↑ 7 @azaozz6 years ago

Replying to Denis-de-Bernardy:

are there any unit tests on wptexturize?

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

comment:11 @Denis-de-Bernardy6 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.

comment:12 @azaozz6 years ago

Also see #3810.

comment:13 @Denis-de-Bernardy6 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.

comment:14 @Denis-de-Bernardy6 years ago

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

comment:15 @janeforshort6 years ago

  • Milestone changed from 2.9 to Future Release

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

comment:16 follow-up: @filosofo5 years ago

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

Let's be realistic.

comment:17 in reply to: ↑ 16 ; follow-up: @Viper007Bond5 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.

comment:18 in reply to: ↑ 17 @Denis-de-Bernardy5 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? :-)

comment:19 @filosofo5 years ago

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

@aliso3 years ago

Quotation mark fix for most recent code in formatting.php

comment:21 @aliso3 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).

@MikeHansenMe3 years ago

screenshot of problem

@MikeHansenMe3 years ago

screenshot after patch

@MikeHansenMe3 years ago

Updated patch path relative from wp folder

comment:22 @MikeHansenMe3 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 3 years ago by MikeHansenMe (previous) (diff)

comment:23 @damst3 years ago

  • Cc damst added

comment:25 @aliso2 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?

comment:26 @nacin20 months ago

  • Keywords wptexturize added

comment:27 @miqrogroove18 months 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.

comment:28 @miqrogroove18 months 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.

comment:29 @miqrogroove18 months ago

#14491 was marked as a duplicate.

comment:30 @miqrogroove18 months ago

#16957 was marked as a duplicate.

@miqrogroove17 months ago

comment:31 @miqrogroove17 months 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.

comment:32 @miqrogroove17 months ago

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

comment:33 @wonderboymusic15 months 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.

comment:34 @miqrogroove15 months 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.

comment:35 @miqrogroove15 months ago

This is fixed now.

comment:36 @samuelsidler15 months ago

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

comment:37 @TobiasBg15 months ago

  • Milestone changed from Future Release to 4.0

comment:39 @helen13 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for investigation.

comment:40 @miqrogroove13 months ago

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

comment:41 @helen13 months ago

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

Apparently this was for numbers-only within quotes.

comment:42 @miqrogroove13 months 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.