Closed
Bug 633210
Opened 13 years ago
Closed 13 years ago
Single hyphens are marked as spelling mistakes
Categories
(Core :: Spelling checker, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b12
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: jrmuizel, Assigned: ehsan.akhgari)
References
Details
(Keywords: regression, Whiteboard: [hardblocker])
Attachments
(2 files)
3.94 KB,
patch
|
smaug
:
review+
brendan
:
review+
beltzner
:
approval2.0+
|
Details | Diff | Splinter Review |
4.19 KB,
patch
|
brendan
:
review+
|
Details | Diff | Splinter Review |
The patch for bug 355178 made single hyphens become marked as spelling mistakes.
Reporter | ||
Updated•13 years ago
|
blocking2.0: --- → ?
Reporter | ||
Updated•13 years ago
|
Blocks: 355178
Keywords: regression
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → ehsan
Assignee | ||
Comment 1•13 years ago
|
||
I screwed up! :(
Attachment #511413 -
Flags: review?(jst)
Attachment #511413 -
Flags: approval2.0?
Assignee | ||
Updated•13 years ago
|
Attachment #511413 -
Flags: review?(Olli.Pettay)
Comment 2•13 years ago
|
||
Comment on attachment 511413 [details] [diff] [review] Patch (v1) Are there still other similar cases?
Attachment #511413 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 3•13 years ago
|
||
(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 :(
Assignee | ||
Updated•13 years ago
|
Attachment #511413 -
Flags: review?(jst)
Assignee | ||
Updated•13 years ago
|
Attachment #511413 -
Flags: approval2.0?
Comment 4•13 years ago
|
||
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 5•13 years ago
|
||
Comment on attachment 511413 [details] [diff] [review] Patch (v1) Lose the else after return and r=me. Thanks, /be
Attachment #511413 -
Flags: review+
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #511471 -
Flags: review?(brendan)
Comment 7•13 years ago
|
||
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+
Assignee | ||
Comment 8•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/05e6ab25d304
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Updated•13 years ago
|
OS: Mac OS X → All
Version: unspecified → Trunk
Updated•13 years ago
|
blocking2.0: ? → ---
Assignee | ||
Comment 11•13 years ago
|
||
Asa, did you clear the blocking flag by mistake?
blocking2.0: --- → ?
Comment 12•13 years ago
|
||
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.
Assignee | ||
Comment 13•13 years ago
|
||
(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.
Description
•