Closed Bug 634153 Opened 9 years ago Closed 9 years ago

Words are incorrectly marked as misspelled when in single quotes and suffixed with a period

Categories

(Core :: Spelling checker, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b12
Tracking Status
blocking2.0 --- -

People

(Reporter: DavidCognito, Assigned: ehsan)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre

Correctly spelled words are marked as misspelled when contained in single quotes and suffixed with a period.

Reproducible: Always

Steps to Reproduce:
1. position cursor in a textarea
2. type "'Apple'."
Actual Results:  
'Apple'. is marked as misspelled.

Expected Results:  
'Apple'. should not be marked as misspelled.
Confirming. Maybe fallout from the scot-free patch? (I haven't looked into it at all)
Status: UNCONFIRMED → NEW
Component: General → Spelling checker
Ever confirmed: true
OS: Windows 7 → All
Product: Firefox → Core
QA Contact: general → spelling-checker
Hardware: x86 → All
Version: unspecified → Trunk
Indeed, a regression from bug 355178.  Seems like hunspell is not as good in tokenization as I thought it was.

Brendan, Beltzner, should I back out bug 355178 at this point?  (yeah, I know what's wrong, and I have the fix in my head, but honestly, this code is too fragile, it's depending on a component which fails to deliver my expectations, and I don't think this is a good point in our release to be experimenting with stuff like this.)
Assignee: nobody → ehsan
Blocks: 355178
Status: NEW → ASSIGNED
blocking2.0: --- → ?
Keywords: regression
Actually, I'm gonna mark it as blocking-final myself.
blocking2.0: ? → final+
What Beltzner said.

/be
I'm assuming he's referring to bug 633210 comment #4
> a=beltzner, but if there's another issue like this, I'm going to sound the
> backout horn.
Discussed in driver call with Ehsan - minusing this, but Ehsan has approval (a=me) for the proposed fix of backing out the busted change, but leaving the added tests.
blocking2.0: final+ → -
Attached patch Patch (v1) (obsolete) — Splinter Review
This patch backs out the code changes in bug 355178 and 633210, and leaves the tests added in those two bugs in the tree.  In addition, it adds tests for the symptoms seen in this bug.

I'm not sure if this really needs review, but asking sicking for a rubberstamp anyways.
Attachment #512557 - Flags: review?(jonas)
Attached patch Patch (v1)Splinter Review
And of course, mark the original "scot-free" test as failing.
Attachment #512557 - Attachment is obsolete: true
Attachment #512562 - Flags: review?(jonas)
Attachment #512557 - Flags: review?(jonas)
http://hg.mozilla.org/mozilla-central/rev/891ce563da54
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
You need to log in before you can comment on or make changes to this bug.