Last Comment Bug 902532 - Spellchecking broken with non-ASCII characters in profile path
: Spellchecking broken with non-ASCII characters in profile path
Status: VERIFIED FIXED
: regression
Product: Core
Classification: Components
Component: Spelling checker (show other bugs)
: 23 Branch
: x86_64 Windows 7
: -- normal with 1 vote (vote)
: mozilla26
Assigned To: Aaron Klotz [:aklotz]
: Ioana (away)
:
Mentors:
: 902976 905133 (view as bug list)
Depends on:
Blocks: 857830
  Show dependency treegraph
 
Reported: 2013-08-07 10:48 PDT by André Bargull
Modified: 2013-09-09 08:14 PDT (History)
18 users (show)
hskupin: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
+
verified
+
verified
+
verified
+
verified


Attachments
Patch v1 (1.74 KB, patch)
2013-08-08 13:04 PDT, Aaron Klotz [:aklotz]
ehsan: review+
bajaj.bhavana: approval‑mozilla‑aurora+
bajaj.bhavana: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑release+
Details | Diff | Splinter Review

Description André Bargull 2013-08-07 10:48:51 PDT
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 Alice0775 White 2013-08-07 12:18:32 PDT
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
Comment 2 Loic 2013-08-07 12:27:57 PDT
Maybe Bug 617897 or bug 857830?
Comment 3 bhavana bajaj [:bajaj] 2013-08-07 13:19:06 PDT
AAron,Orva : any idea whats going on here ?
Comment 4 Alice0775 White 2013-08-08 02:29:48 PDT
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
Comment 5 Aaron Klotz [:aklotz] 2013-08-08 08:28:38 PDT
I see what the problem is. Working on a fix...
Comment 6 Martín 2013-08-08 09:52:51 PDT
*** Bug 902976 has been marked as a duplicate of this bug. ***
Comment 7 Aaron Klotz [:aklotz] 2013-08-08 13:04:50 PDT
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.
Comment 8 :Ehsan Akhgari 2013-08-08 13:23:17 PDT
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.  ;-)
Comment 10 Martín 2013-08-09 08:02:50 PDT
I don't find the file that has to be modified. ¿Which is the path of the file?

Thank you very much.
Comment 11 Alice0775 White 2013-08-09 08:54:33 PDT
(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 Martín 2013-08-09 09:03:44 PDT
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.
Comment 13 André Bargull 2013-08-09 09:34:53 PDT
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! ;-)
Comment 14 Pavel Cvrcek [:JasnaPaka] 2013-08-09 12:40:12 PDT
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.
Comment 15 :Ehsan Akhgari 2013-08-09 14:46:34 PDT
(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.
Comment 16 :Ehsan Akhgari 2013-08-09 14:47:02 PDT
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
Comment 18 Ryan VanderMeulen [:RyanVM] 2013-08-09 16:49:05 PDT
https://hg.mozilla.org/mozilla-central/rev/f0de2d95b46b
Comment 19 Michal Stanke (Mozilla.cz) [:MikkCZ] 2013-08-10 01:57:38 PDT
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.
Comment 20 Ryan VanderMeulen [:RyanVM] 2013-08-10 03:53:28 PDT
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 Martín 2013-08-10 09:45:06 PDT
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.
Comment 22 :Ehsan Akhgari 2013-08-12 07:02:10 PDT
(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 Martín 2013-08-12 07:45:33 PDT
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.
Comment 24 :Ehsan Akhgari 2013-08-12 09:26:37 PDT
(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!
Comment 25 Lukas Blakk [:lsblakk] use ?needinfo 2013-08-12 15:52:48 PDT
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.
Comment 26 Ryan VanderMeulen [:RyanVM] 2013-08-13 04:55:45 PDT
https://hg.mozilla.org/releases/mozilla-release/rev/da8c3f3acd56
Comment 27 pejanssens 2013-08-14 08:24:44 PDT
*** Bug 905133 has been marked as a duplicate of this bug. ***
Comment 28 Ioana (away) 2013-08-16 01:16:56 PDT
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.
Comment 29 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-08-16 12:16:48 PDT
Henrik, is this something we could have covered for future releases via automation?
Comment 30 Henrik Skupin (:whimboo) 2013-08-18 22:49:36 PDT
(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.
Comment 31 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-08-19 11:27:45 PDT
Thanks Henrik, let's discuss this further offline.
Comment 32 Paul Silaghi, QA [:pauly] 2013-08-20 04:25:49 PDT
(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.
Comment 33 Ioana (away) 2013-09-09 08:14:27 PDT
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)

Note You need to log in before you can comment on or make changes to this bug.