Closed Bug 633210 Opened 10 years ago Closed 10 years ago

Single hyphens are marked as spelling mistakes

Categories

(Core :: Spelling checker, defect)

x86
All
defect
Not set
normal

Tracking

()

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

People

(Reporter: jrmuizel, Assigned: ehsan)

References

Details

(Keywords: regression, Whiteboard: [hardblocker])

Attachments

(2 files)

The patch for bug 355178 made single hyphens become marked as spelling mistakes.
blocking2.0: --- → ?
Blocks: 355178
Keywords: regression
Assignee: nobody → ehsan
Attached patch Patch (v1)Splinter Review
I screwed up!  :(
Attachment #511413 - Flags: review?(jst)
Attachment #511413 - Flags: approval2.0?
Attachment #511413 - Flags: review?(Olli.Pettay)
Comment on attachment 511413 [details] [diff] [review]
Patch (v1)

Are there still other similar cases?
Attachment #511413 - Flags: review?(Olli.Pettay) → review+
(In reply to comment #2)
> Comment on attachment 511413 [details] [diff] [review]
> Patch (v1)
> 
> Are there still other similar cases?

Not to the best of my knowledge, but I didn't catch this problem before pushing either :(
Attachment #511413 - Flags: review?(jst)
Attachment #511413 - Flags: approval2.0?
Comment on attachment 511413 [details] [diff] [review]
Patch (v1)

a=beltzner, but if there's another issue like this, I'm going to sound the backout horn.

Sound good?
Attachment #511413 - Flags: approval2.0+
Comment on attachment 511413 [details] [diff] [review]
Patch (v1)

Lose the else after return and r=me. Thanks,

/be
Attachment #511413 - Flags: review+
Attached patch Patch (v2)Splinter Review
Attachment #511471 - Flags: review?(brendan)
Comment on attachment 511471 [details] [diff] [review]
Patch (v2)

>+    if (aIndex > 0 &&
>+        mDOMWordText[aIndex] == '-' &&
>+        mDOMWordText[aIndex - 1] != '-' &&
>+        ClassifyCharacter(aIndex - 1, false) == CHAR_CLASS_WORD) {

A one-line comment here about requiring a word-char after would be good.

>+      if (aIndex == PRInt32(mDOMWordText.Length()) - 1)
>+        return CHAR_CLASS_SEPARATOR;
>+      if (ClassifyCharacter(aIndex + 1, false) == CHAR_CLASS_WORD)
>+        return CHAR_CLASS_WORD;
>     }
>+    return CHAR_CLASS_SEPARATOR;
>   }
> 
>   // any other character counts as a word
>   return CHAR_CLASS_WORD;
> }

r=me with that.

/be
Attachment #511471 - Flags: review?(brendan) → review+
http://hg.mozilla.org/mozilla-central/rev/05e6ab25d304
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Duplicate of this bug: 633306
OS: Mac OS X → All
Version: unspecified → Trunk
Duplicate of this bug: 633443
blocking2.0: ? → ---
Asa, did you clear the blocking flag by mistake?
blocking2.0: --- → ?
Ehasan, I removed the nomination because the bug history suggested that this bug had already been fixed for Mozilla2/Firefox 4. Has it not landed for Mozill2 / Firefox 4? Perhaps it should be re-opened then instead of Resolved Fixed.
(In reply to comment #12)
> Ehasan, I removed the nomination because the bug history suggested that this
> bug had already been fixed for Mozilla2/Firefox 4. Has it not landed for
> Mozill2 / Firefox 4? Perhaps it should be re-opened then instead of Resolved
> Fixed.

No, it has been fixed.  But that has nothing to do with it blocking or not.

FWIW, we sometimes mark FIXED bugs as blocking too.  What this means is that we wouldn't release without that particular bug being fixed.  This helps to make sure that if the patch was to be backed out for whatever reason (and therefore the bug to be reopened), the original issue doesn't fall off track in drivers' queries.

Hope that this explains why the blockig2.0? flag needs to remain set here.  :-)
blocking2.0: ? → final+
Whiteboard: [hardblocker]
You need to log in before you can comment on or make changes to this bug.