André Bargull [:anba] Reporter
	Description • 11 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.

	Alice0775 White
	Comment 1 • 11 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

	Loic
	Comment 2 • 11 years ago

Maybe Bug 617897 or bug 857830?

	bhavana bajaj [:bajaj]
	Comment 3 • 11 years ago

AAron,Orva : any idea whats going on here ?

	Alice0775 White
	Comment 4 • 11 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

	(No longer employed by Mozilla) Aaron Klotz Assignee
	Comment 5 • 11 years ago

I see what the problem is. Working on a fix...

	(No longer employed by Mozilla) Aaron Klotz Assignee
	Comment 7 • 11 years ago

This patch replaces NS_ConvertUTF8toUTF16 with NS_CopyNativeToUnicode in the Windows case, as the provided filename is native, not UTF-8.

	(no longer active)
	Comment 8 • 11 years ago

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.  ;-)

	(No longer employed by Mozilla) Aaron Klotz Assignee
	Comment 9 • 11 years ago

Try build:
https://tbpl.mozilla.org/?tree=Try&rev=97277273c933

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

	Martín
	Comment 10 • 11 years ago

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

Thank you very much.

	Alice0775 White
	Comment 11 • 11 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/

	Martín
	Comment 12 • 11 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.

	André Bargull [:anba] Reporter
	Comment 13 • 11 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! ;-)

	Pavel Cvrcek [:JasnaPaka]
	Comment 14 • 11 years ago

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.

	(no longer active)
	Comment 15 • 11 years ago

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

	(no longer active)
	Comment 16 • 11 years ago

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

	(No longer employed by Mozilla) Aaron Klotz Assignee
	Comment 17 • 11 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/381b423ec65b
https://hg.mozilla.org/releases/mozilla-beta/rev/bb7cb19f359a

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/f0de2d95b46b

	Michal Stanke (Mozilla.cz) [:mstanke][:MikkCZ]
	Comment 19 • 11 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 20 • 11 years ago

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.

	Martín
	Comment 21 • 11 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.

	(no longer active)
	Comment 22 • 11 years ago

(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?

	Martín
	Comment 23 • 11 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.

	(no longer active)
	Comment 24 • 11 years ago

(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!

	Lukas Blakk [:lsblakk] use ?needinfo
	Comment 25 • 11 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 26 • 11 years ago

https://hg.mozilla.org/releases/mozilla-release/rev/da8c3f3acd56

	Ioana (away)
	Comment 28 • 11 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.

	u279076
	Comment 29 • 11 years ago

Henrik, is this something we could have covered for future releases via automation?

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 30 • 11 years ago

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

	u279076
	Comment 31 • 11 years ago

Thanks Henrik, let's discuss this further offline.

	Paul Silaghi, QA [:pauly]
	Comment 32 • 11 years ago

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

	Ioana (away)
	Comment 33 • 11 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)