Upgrade to Hunspell 1.4.1

RESOLVED FIXED in Firefox 47

Status

()

Core
Spelling checker
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: RyanVM, Assigned: RyanVM)

Tracking

Trunk
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed, firefox48 fixed, firefox49 fixed, firefox-esr4547+ fixed)

Details

Attachments

(2 attachments)

(Assignee)

Description

2 years ago
Hunspell 1.4.0 had a couple bugs that led to a 1.4.1 release shortly after we'd landed the 1.4.0 update. I'd like to update to 1.4.1 before uplifting it all around.
(Assignee)

Comment 1

2 years ago
Created attachment 8748447 [details] [diff] [review]
update Hunspell to version 1.4.1

Aryeh, I hear you're getting more involved in Editor-related things these days. Can I interest you in getting your feet wet with a small Hunspell update?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e7fd29e08bf
Attachment #8748447 - Flags: review?(ayg)
Comment hidden (obsolete)
(Assignee)

Comment 3

2 years ago
Comment on attachment 8748447 [details] [diff] [review]
update Hunspell to version 1.4.1

Except I was looking at the wrong Try push at the time. *sigh*

The correct Try push for this update passes tests just fine, I swear!
Attachment #8748447 - Flags: review?(ayg)
Comment on attachment 8748447 [details] [diff] [review]
update Hunspell to version 1.4.1

I'm not really sure I'm qualified to review this, sorry.
Attachment #8748447 - Flags: review?(ayg)
(Assignee)

Updated

2 years ago
Attachment #8748447 - Flags: review?(ehsan)
For the purposes of this test I have used the Bugzilla textbox, the one from GitHub, the Facebook “Post” textbox, the Twitter “Tweet” textbox, the text box from wordcounter.net and a basic one created in HTML.

Tested on the following platforms:
Windows 10 x64
Ubuntu 14.04 x32
Mac OS X 10.11

The following dictionaries were used during testing:
English (US)
English (UK)
Romanian
Hindi
Spanish

Conclusions:
I haven't observed any odd behavior after the Hunspell update. All text boxes behave the same way on the tested platforms. Worked correctly for most of the cases with respect to correctly spelled and misspelled words. 

However, Romanian, Spanish and Hindi words using special characters are not correctly flagged by the spell checker (English US dictionary) as incorrect.
Whenever a number or special character is appended to any word, spell check ignores it and it is not flagged. This is contrary to behavior seen in Chrome browser.

For the first issue I will log a separate bug, but I'm not sure if the second one is a valid one. What are your thoughts on this?
Flags: needinfo?(ryanvm)
(Assignee)

Comment 6

2 years ago
Thanks for all the testing!

(In reply to Ciprian Muresan [:cmuresan] from comment #5)
> Whenever a number or special character is appended to any word, spell check
> ignores it and it is not flagged.

Certainly sounds like a bug to me.
Flags: needinfo?(ryanvm)

Updated

2 years ago
Attachment #8748447 - Flags: review?(ehsan) → review+

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f9ba1836d5cd
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(Assignee)

Comment 9

2 years ago
Created attachment 8753977 [details] [diff] [review]
roll-up patch for uplift

Approval Request Comment
[Feature/regressing bug #]: n/a
[User impact if declined]: numerous issues found by Coverity and fuzzing (see bug 1257902 deps)
[Describe test coverage new/current, TreeHerder]: v1.4.0 has been on trunk for over two weeks now. The v1.4.1 update has baked for over a week now without issue. Also had a QA pass with multiple languages and no new issues found. We also have automated spellchecker tests in CI. No obvious crash spikes on crash-stats that I can see.
[Risks and why]: Fuzzing hasn't turned up any new issues and no known regressions in 2+ weeks on trunk makes me feel pretty confident about this being low risk.
[String/UUID change made/needed]: none
Attachment #8753977 - Flags: approval-mozilla-esr45?
Attachment #8753977 - Flags: approval-mozilla-beta?
Attachment #8753977 - Flags: approval-mozilla-aurora?
Comment on attachment 8753977 [details] [diff] [review]
roll-up patch for uplift

Spellchecker update after almost a year (incl. sec fixes from those guys), no regressions found in 2 weeks of baking on central. Aurora48+, Beta47+, ESR45+
Attachment #8753977 - Flags: approval-mozilla-esr45?
Attachment #8753977 - Flags: approval-mozilla-esr45+
Attachment #8753977 - Flags: approval-mozilla-beta?
Attachment #8753977 - Flags: approval-mozilla-beta+
Attachment #8753977 - Flags: approval-mozilla-aurora?
Attachment #8753977 - Flags: approval-mozilla-aurora+

Updated

2 years ago
status-firefox47: --- → affected
status-firefox48: --- → affected
status-firefox-esr45: --- → affected

Updated

2 years ago
tracking-firefox-esr45: --- → 47+

Comment 11

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/3a4e50717846
status-firefox48: affected → fixed

Comment 12

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/57f15bb73e74
status-firefox47: affected → fixed

Comment 13

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr45/rev/d69ba0ebefa7
status-firefox-esr45: affected → fixed
Blocks: 1256493
Blocks: 1256739
Blocks: 1256968
(Assignee)

Updated

2 years ago
Blocks: 1318955
You need to log in before you can comment on or make changes to this bug.