Spellchecking broken with non-ASCII characters in profile path

VERIFIED FIXED in Firefox 23

Status

()

Core
Spelling checker
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: André Bargull, Assigned: aklotz)

Tracking

({regression})

23 Branch
mozilla26
x86_64
Windows 7
regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox22 unaffected, firefox23+ verified, firefox24+ verified, firefox25+ verified, firefox26+ verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (Beta/Release)
Build ID: 20130730113002

Steps to reproduce:

1) Create a new profile and make sure non-ASCII characters are present in the path to the profile directory, e.g. in my case "André"
2) Install a dictionary, e.g. EN-US
3) And finally insert text into an input element and start spell checking, e.g. "Test spelling"

This is a new regression since Firefox 23. Thunderbird 17.0.8 is not affected, therefore I've filed it under the Firefox product category instead of Core. 
Does not reproduce under Ubuntu 12.04, only reproducible for Windows 7 and Windows 8.


Actual results:

All words are underlined in red as misspelled.


Expected results:

Only misspelled words are marked as such.

Comment 1

4 years ago
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/b25afb305360
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130507 Firefox/23.0 ID:20130507190734
Bad:
http://hg.mozilla.org/mozilla-central/rev/b980d32c366f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130507 Firefox/23.0 ID:20130507191135
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=b25afb305360&tochange=b980d32c366f
Status: UNCONFIRMED → NEW
status-firefox22: --- → unaffected
status-firefox23: --- → affected
tracking-firefox24: --- → ?
tracking-firefox25: --- → ?
tracking-firefox26: --- → ?
Component: Untriaged → Spelling checker
Ever confirmed: true
Keywords: regression
Product: Firefox → Core

Comment 2

4 years ago
Maybe Bug 617897 or bug 857830?
AAron,Orva : any idea whats going on here ?
Flags: needinfo?(iivari.aikas)
Flags: needinfo?(aklotz)

Comment 4

4 years ago
In local build
Last Good: e1f30dbb8108
First Bad: abaf789d8030
Regressed by:
	abaf789d8030	Aaron Klotz — Bug 857830: Part 2 - Adds readahead to read-only fopen calls in Hunspell. r=ehsan
Blocks: 857830
I see what the problem is. Working on a fix...
Flags: needinfo?(aklotz)
Assignee: nobody → aklotz
Status: NEW → ASSIGNED

Updated

4 years ago
Duplicate of this bug: 902976
Created attachment 787739 [details] [diff] [review]
Patch v1

This patch replaces NS_ConvertUTF8toUTF16 with NS_CopyNativeToUnicode in the Windows case, as the provided filename is native, not UTF-8.
Attachment #787739 - Flags: review?(ehsan)
Comment on attachment 787739 [details] [diff] [review]
Patch v1

Review of attachment 787739 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/spellcheck/hunspell/src/hunspell_fopen_hooks.h
@@ +43,5 @@
>    int fd = -1;
>  #if defined(XP_WIN)
> +  nsAutoString utf16Filename;
> +  nsresult rv = NS_CopyNativeToUnicode(nsDependentCString(filename),
> +                                       utf16Filename);

Please add a comment why this is needed.  Feel free to add hate words for Windows.  ;-)
Attachment #787739 - Flags: review?(ehsan) → review+

Updated

4 years ago
Flags: needinfo?(iivari.aikas)

Updated

4 years ago
status-firefox24: --- → affected
status-firefox25: --- → affected
status-firefox26: --- → affected
tracking-firefox24: ? → +
tracking-firefox25: ? → +
tracking-firefox26: ? → +
Try build:
https://tbpl.mozilla.org/?tree=Try&rev=97277273c933

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0de2d95b46b

Comment 10

4 years ago
I don't find the file that has to be modified. ¿Which is the path of the file?

Thank you very much.

Comment 11

4 years ago
(In reply to Martín from comment #10)
> I don't find the file that has to be modified. ¿Which is the path of the
> file?
> 
> Thank you very much.

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/aklotz@mozilla.com-97277273c933/try-win32/

Comment 12

4 years ago
Sorry, I think that I have explained bad. I refered to a local path to modify the file in the PC. ¿It is possible fix the problem only modifying a file in the local file system or it is necessary build Firefox?


Thank you very much.
(Reporter)

Comment 13

4 years ago
It is necessary to rebuild Firefox.

A couple of workaround are possible, though. Since this bug only occurs when there are certain special characters in the path to your Firefox profile, you need to make sure the profile path contains only simple ASCII characters (cf. http://en.wikipedia.org/wiki/ASCII).

(1) Create a new profile and manually set the profile folder in step 2 of the Profile Creation Wizard, e.g. to C:\Firefox-Profiles\NewProfile

(2) Move your profile folder to different path, see http://kb.mozillazine.org/Moving_your_profile_folder

(3) Use Windows junctions and change manually change C:\Users\<USER-NAME>\AppData\Roaming\Mozilla\Firefox\profiles.ini to use to the junction, e.g. a junction for C:\Users\André to C:\Users\Andre 

As usual, you should only use one of those workarounds if you know what you're doing, because I don't want to be responsible if any of your profile data gets corrupted! ;-)
Big issue for Czech (cs) users. We have received many reports about it. Is minor fix for Firefox 23 possible? Workaround is not good for common users.
(In reply to Pavel Cvrcek (Mozilla.cz) [:JasnaPaka] from comment #14)
> Big issue for Czech (cs) users. We have received many reports about it. Is
> minor fix for Firefox 23 possible? Workaround is not good for common users.

Release managers, I heard chatter about doing a dot release for 23.  Any chance we could take this as a ride-along?  It's almost zero-risk.
Flags: needinfo?(release-mgmt)
Comment on attachment 787739 [details] [diff] [review]
Patch v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 857830
User impact if declined: comment 0
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): zero risk
String or IDL/UUID changes made by this patch: none
Attachment #787739 - Flags: approval-mozilla-beta?
Attachment #787739 - Flags: approval-mozilla-aurora?

Updated

4 years ago
Attachment #787739 - Flags: approval-mozilla-beta?
Attachment #787739 - Flags: approval-mozilla-beta+
Attachment #787739 - Flags: approval-mozilla-aurora?
Attachment #787739 - Flags: approval-mozilla-aurora+

Updated

4 years ago
tracking-firefox23: --- → ?
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-aurora/rev/381b423ec65b
https://hg.mozilla.org/releases/mozilla-beta/rev/bb7cb19f359a
Keywords: checkin-needed
status-firefox24: affected → fixed
status-firefox25: affected → fixed
https://hg.mozilla.org/mozilla-central/rev/f0de2d95b46b
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox26: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
I still see mozilla26 as a target milestone. Will there be any fixing update for version 23? Personally I am not faced to deal with this bug, but still many users around me complain (Czech is full of diacritics).

TB 17.0.8 is unaffected, but next 24 version would be as well as Firefox.
The target milestone only refers to when this patch landed on trunk. It has already been uplifted for version 24 per comment 17. No decision has been made yet about shipping it with a version 23 bugfix release.

Comment 21

4 years ago
I want to comment that my local path only contains ASCII characters, although it contains a non NVT ASCII character (í). I suppose that we refer to non NVT ASCII characters instead of non ASCII characters, but I want to be sure that the proble m is completely fixed.

Thanks.
(In reply to Martín from comment #21)
> I want to comment that my local path only contains ASCII characters,
> although it contains a non NVT ASCII character (í). I suppose that we refer
> to non NVT ASCII characters instead of non ASCII characters, but I want to
> be sure that the proble m is completely fixed.
> 
> Thanks.

What do you mean by NVT?

Comment 23

4 years ago
I refer to 7-bit ASCII code. The 8-bit ASCII code includes the character 'í' that is in my path. I have seen that ASCII generally means the 7-bit ASCII (US-ASCII), in Spanish often we indicate it.

Sorry for the confussion.
(In reply to comment #23)
> I refer to 7-bit ASCII code. The 8-bit ASCII code includes the character 'í'
> that is in my path. I have seen that ASCII generally means the 7-bit ASCII
> (US-ASCII), in Spanish often we indicate it.

Oh you probably mean the non-Unicode system codepage setting.  If that is the case, this patch probably fixes your problem.  At any rate, I encourage you to download a Nightly build (http://nightly.mozilla.org/) and test it out.  Thanks!
tracking-firefox23: ? → +
Comment on attachment 787739 [details] [diff] [review]
Patch v1

[Triage Comment]
Ryan - let's take this for our 23.0.1 respin as a low risk ride-along.
Attachment #787739 - Flags: approval-mozilla-release+
Flags: needinfo?(release-mgmt)
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-release/rev/da8c3f3acd56
status-firefox23: affected → fixed
Keywords: checkin-needed

Updated

4 years ago
Duplicate of this bug: 905133
Keywords: verifyme

Comment 28

4 years ago
Verified as fixed on:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (20130815181913)

Misspelled words are still underlined, but those that are spelled correctly are not underlined anymore.
status-firefox23: fixed → verified
QA Contact: ioana.budnar
Henrik, is this something we could have covered for future releases via automation?
Flags: needinfo?(hskupin)
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #29)
> Henrik, is this something we could have covered for future releases via
> automation?

Sadly not as of now. Mozmill tests are purely written in JavaScript and doesn't let us control the creation of the profile. In general it would be possible but would require a fair amount of work. So not sure when we will have that feature. But I'm sure it will be possible to write a Marionette test for that regression.
Flags: needinfo?(hskupin) → in-testsuite?
Thanks Henrik, let's discuss this further offline.
(In reply to Ioana Budnar, QA [:ioana] from comment #28)
> Verified as fixed on:
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0
> (20130815181913)
> 
> Misspelled words are still underlined, but those that are spelled correctly
> are not underlined anymore.
Verified fixed FF 24b4 Win 7 x64.
status-firefox24: fixed → verified

Comment 33

4 years ago
Verified as fixed on 
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 (20130908004001)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 (20130909030204)
Status: RESOLVED → VERIFIED
status-firefox25: fixed → verified
status-firefox26: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.