Single hyphens are marked as spelling mistakes

RESOLVED FIXED in mozilla2.0b12

Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jrmuizel, Assigned: Ehsan)

Tracking

({regression})

Trunk
mozilla2.0b12
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [hardblocker])

Attachments

(2 attachments)

Reporter

Description

8 years ago
The patch for bug 355178 made single hyphens become marked as spelling mistakes.
Reporter

Updated

8 years ago
blocking2.0: --- → ?
Reporter

Updated

8 years ago
Blocks: 355178
Keywords: regression
Assignee

Updated

8 years ago
Assignee: nobody → ehsan
Assignee

Comment 1

8 years ago
Posted patch Patch (v1)Splinter Review
I screwed up!  :(
Attachment #511413 - Flags: review?(jst)
Attachment #511413 - Flags: approval2.0?
Assignee

Updated

8 years ago
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+
Assignee

Comment 3

8 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

8 years ago
Attachment #511413 - Flags: review?(jst)
Assignee

Updated

8 years ago
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+
Assignee

Comment 6

8 years ago
Posted 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+
Assignee

Comment 8

8 years ago
http://hg.mozilla.org/mozilla-central/rev/05e6ab25d304
Status: NEW → RESOLVED
Last Resolved: 8 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

Updated

8 years ago
blocking2.0: ? → ---
Assignee

Comment 11

8 years ago
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.
Assignee

Comment 13

8 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.