409796 - No pictures shown in saved file (file name and folder name, containing that file, is in cyrillic)

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Alexander L. Slovesnik]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.9b3pre) Gecko/2007122304 Minefield/3.0b3pre
Build Identifier: 

No pictures are shown in saved file, if file name and folder name, containing that file, is in cyrillic.

Reproducible: Always

Steps to Reproduce:
1.Create a folder on disk with cyrillic name (for example C:\тестовый)
2.Save a file in this folder (choosing Web Page, complete) and name it with cyrillic letters (for example тестовый.html)
3.Open saved file
Actual Results:  
Saved file is shown without any saved pictures and any stylesheet

Expected Results:  
Saved file is shown up with pictures and proper style

This bug is Windows only (at least it doesn't show up in my Ubuntu 7.10). I've reproduced this bug on Windows XP SP2 with Firefox 3.0b2.
If you change name of folder with saved file from cyrillic to latin, bug dissappears. So page is saved correctly, Firefox just doesn't show pictures, if path to file contains any cyrillic letters.

Comment 1

[Alexander L. Slovesnik]

So I've found regression window:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2006/05/2006-05-01-04-mozilla1.8/firefox-2.0a1.en-US.win32.installer.exe build works
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2006/05/2006-05-02-03-mozilla1.8/firefox-2.0a1.en-US.win32.installer.exe build broken
Looking at regression window, I guess that this bug is regression from Bug 278161
Adding dependency, cc'ing Bug 278161 owner, moving to right component and asking for blocking 1.9.

Comment 2

[Mike Schroepfer]

(In reply to comment #1)
> So I've found regression window:
> ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2006/05/2006-05-01-04-mozilla1.8/firefox-2.0a1.en-US.win32.installer.exe
> build works
> ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2006/05/2006-05-02-03-mozilla1.8/firefox-2.0a1.en-US.win32.installer.exe
> build broken
> Looking at regression window, I guess that this bug is regression from Bug
> 278161
> Adding dependency, cc'ing Bug 278161 owner, moving to right component and
> asking for blocking 1.9.
> 

Based on that range this issue exists in FF2.0.0.x right? 

Comment 3

[Alexander L. Slovesnik]

(In reply to comment #2)
> (In reply to comment #1)
> > So I've found regression window:
> > ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2006/05/2006-05-01-04-mozilla1.8/firefox-2.0a1.en-US.win32.installer.exe
> > build works
> > ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2006/05/2006-05-02-03-mozilla1.8/firefox-2.0a1.en-US.win32.installer.exe
> > build broken
> > Looking at regression window, I guess that this bug is regression from Bug
> > 278161
> > Adding dependency, cc'ing Bug 278161 owner, moving to right component and
> > asking for blocking 1.9.
> > 
> 
> Based on that range this issue exists in FF2.0.0.x right? 
> 
Yes.

Comment 4

[Mike Schroepfer]

Given comment 3 moving to wanted list

Comment 5

[Evgeniy Ivanov]

I have 3.0b3pre. Everything seems to be ok.
There is a bug with showing cyrillic names after coping in the address bar, but just in showing.
For example, I create New Folder (Russian translation).
Then I save http://www.linuxwallpapers.org/linux-wallpapers.htm
After I open saved html. It's ok. Also I may open any image (copy its address and paste to the address bar).

Comment 6

[Evgeniy Ivanov]

Sorry, I've saved file with English name.

Comment 7

[Evgeniy Ivanov]

As I think, the problem is in saving pages. And also it's a kind of windows problems.
See the example.
Let's save a simple page with picture. And give it a cyrillic name.
http://linuxwallpapers.org/linux/slides/22302-ukGinger-20.html
As the result we will have CyrillicName.html file and CyrillicName_files directory. Images (and also xml-s,css-s) are linked with src="CyrillicName_files/FILE.*.
But saving with ff I see not CyrillicName_files, but the thing similar to %D0%BA%D0%B4%D0%B5_files, where "%D0%BA%D0%B4%D0%B5_files" is url-encoded utf-8 string representation. I'm not strong in it, but in Windows this thing strongly depends from the charset used in the <header>. Also it doesn't wont to open links with more than 1 such things in the path. Also the CyrillicName_files shouldn't be in the utf-8 if the page is non-utf-8 (I'm not sure, so it's the question to W3C DOM).
I tried to save the same page with IE in utf-8, I got the same html, but with correct CyrillicName_files links. FF opens it correctly. Also if change the page 's links ti utf-8 (and the header charset too) everything works too.

The problem is in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp
It fixes links to the local saved files. But it uses utf-8 for Cyrillic names.
I will work with this the whole next week (I will have the cool thing named vocation).

Comment 8

[Timur Timirkhanov]

This is a very strange bug...

Firefox 3.0b3pre on Windows XP SP2 rus (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2007122905 Minefield/3.0b3pre) DOES show pictures when the file is saved in charset equal to system locale (i.e. when the file is saved in win-1251), but does NOT show pictures when the file is saved in charset NOT equal to system locale (e.g. when the file is saved in koi8-r).

Firefox 2.0.0.2 on Windows XP SP2 rus (Mozilla/5.0 (Windows; U; Windows NT 5.1; ru; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11) does NOT show pictures in either case.

Firefox 2.0.0.2 on SUSE SLED 10 SP1 rus (Mozilla/5.0 (X11; U; Linux i686; ru; rv:1.8.1.2) Gecko/20060911 SUSE/2.0.0.2-2.13 Firefox/2.0.0.2) behaves the same way as Firefox 3.0b3pre on Windows XP SP2 rus.

Comment 9

[Evgeniy Ivanov]

(In reply to comment #8)
> This is a very strange bug...

Nothing strange :) If you look saved html you will see that the links (I mean the src="" strings) are a kind of broken. They're saved in a wrong way (all in utf-8).
First of all I will try to make it saving all in utf-8 and then to fix the whole problem.

The open routines are the second place to have a look.

I will work with this bug.

Comment 10

[Evgeniy Ivanov]

If you diff the htmls of the pages saved in linux and windows you will see, that the "src=" attributes are different. In windows they are in escaped windows-1251 (as I think), and in linux they are escaped as utf-8.
You may easily check it using this article: http://www.w3.org/International/O-URL-code.html

So, when I will find how it's encoded I will fix the bug.

Comment 11

[Evgeniy Ivanov]

Oops, as always forgot to add.
When ff opens files (while loading a page) it uses dirs escaped in utf8, and the DIR_files from the attribute escaped in windows-1251. So that's why when the directory is Latin everything is ok (the attribute is unescaped like win-1251, but the directory isn't).

Comment 12

[Timur Timirkhanov]

If you save a page with koi8-r encoding, the path won't be in windows-1251 or utf-8, they'll be in koi8-r.

Comment 13

[Evgeniy Ivanov]

PAGE_files will be encoded in koi8-r, but the full path will be encoded in utf8 (it is used to load the page when open it), so that's why the problem disappears if there is Latin path. In Linux any links are saved in escaped utf8.

Comment 14

[Evgeniy Ivanov]

I've found very interesting discusion (the have found the same as I).
It's in Russian: http://forum.mozilla-russia.org/viewtopic.php?id=21194&p=1

Comment 15

[Vialent]

See the https://addons.mozilla.org/ru/firefox/addon/4723 extension. Named as "Save Complete". I think that this extension solves this bug. And this not only for russian language, see language list for this extension.
I think, that 400$ you must send to Save Complete's author (warhammerkid) . :) 

Comment 16

[Evgeniy Ivanov]

Attachment: fixes the problem. try1.

This patch should fix the problem.
Thanks biezi for pointing me back to nsWebBrowserPersist.cpp. I thought I checked everything therem but he was right I should view only this file. If not him I would brake everything in URLHelpers and LocalFileWin before returning to it.
I hope nothing bad would happen from this patch.

Comment 18

[Evgeniy Ivanov]

Build for ui-review.
http://rapidshare.com/files/91528025/firefox-3.0b2.en-US.win32.zip

Comment 19

[Evgeniy Ivanov]

I've compared mochitest results: everything is ok.

Comment 20

[Evgeniy Ivanov]

Attachment: For ff2

Comment 21

[Timur Timirkhanov]

powerfox,
you should ask for a review, superreview and approval-1.8.1 to land your patch on Firefox 2, if you want to.

Comment 22

[Evgeniy Ivanov]

Comment on attachment 304732 [details] [diff] [review]
For ff2

Ok, done.

Comment 23

[Timur Timirkhanov]

powerfox,
now you should ask for an approval-1.8.1 and approval-1.9.

Comment 24

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 301753 [details] [diff] [review]
fixes the problem. try1.

a=beltzner

Comment 25

[Reed Loden [:reed]]

Checking in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp;
/cvsroot/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp,v  <--  nsWebBrowserPersist.cpp
new revision: 1.134; previous revision: 1.133
done

Comment 26

[Timur Timirkhanov]

I think, this bag should be REOPENED as pages saved in FF trunk with it fixed do not open in IE 5, 6, or 7, which ae still are used, at least, by more than 80% of the Internet users.

Comment 27

[Alexander L. Slovesnik]

(In reply to comment #26)
> I think, this bag should be REOPENED as pages saved in FF trunk with it fixed
> do not open in IE 5, 6, or 7, which ae still are used, at least, by more than
> 80% of the Internet users.
> 
Please read STR in comment #0. There is no mention of IE in them. This bug was about opening pages, saved in Firefox/SeaMonkey, with Firefox/SeaMonkey, and this bug is fixed.
Anyway best practice is to file new bug for new issue. 

Comment 28

[Evgeniy Ivanov]

(In reply to comment #26)
> I think, this bag should be REOPENED as pages saved in FF trunk with it fixed
> do not open in IE 5, 6, or 7, which ae still are used, at least, by more than
> 80% of the Internet users.
> 

Of course such bug should be fixed. But it's just a kind of regression (another bug) from this patch: filenames (the whole text of the .html) should be encoded in the codepage specified in the meta of the document.
And this bug was caused by getting the path in the native charset.

If you file new bug I will fix it (but please put the link here).

Comment 29

[Daniel Veditz [:dveditz]]

Comment on attachment 304732 [details] [diff] [review]
For ff2

Are there unit tests for this (trunk) to make sure nothing breaks? Since this is a regression from an older fix we are nervous about re-breaking things (test should cover this bug and ensure 278161 remains working).

Comment 30

[Evgeniy Ivanov]

Daniel: mochitests are ok. And I think another tests to (patch was commited).
From header of Bug 278161: "make file URLs always be in (escaped) UTF-8 regardless of the file system encoding". The bug reason was in that ff created files with system encoding and not UTF-8, so I think you shouldn't worry.
Of course I'm not authoritative person you may listen to.
But you may be interested in the Bug 419594.

Comment 31

[Daniel Veditz [:dveditz]]

Branch drivers are nervous about the regression bug 419594. Standing on "not lossy" principles only goes so far if it's not web compatible. Is there a standard that says file: uris can be or should be utf8? More naturally I believe file: uris are interpreted in the native file character set.

We're not going to approve this for branch until the regression issue is resolved.

Comment 32

[Evgeniy Ivanov]

Daniel, I don't understand why you think bug 419594 is the regression. I have the same problems with ff without this patch. It's the separate problem about the same thing.

Heh, it seems that opera has the same problem as described in this bugreport.

Comment 33

[Alexander L. Slovesnik]

(In reply to comment #31)
> Branch drivers are nervous about the regression bug 419594.
As reporter of this bug I can confirm that Bug 419594 is not regression from this bug. It is different issue and it exists for the long time.

Comment 34

[Daniel Veditz [:dveditz]]

Comment on attachment 304732 [details] [diff] [review]
For ff2

approved for 1.8.1.13, a=dveditz for release-drivers

Comment 35

[Evgeniy Ivanov]

checkin-needed for 1.8.1.13 (attachment 304732 [details] [diff] [review]).

Comment 36

[Reed Loden [:reed]]

MOZILLA_1_8_BRANCH:

Checking in embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp;
/cvsroot/mozilla/embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp,v  <--  nsWebBrowserPersist.cpp
new revision: 1.109.8.5; previous revision: 1.109.8.4
done

Comment 37

[Al Billings [:abillings - ex-MoCo]]

I've verified the bug and the fix in Windows XP. Fix verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/2008031114 Firefox/2.0.0.13. 

Comment 38

[Al Billings [:abillings - ex-MoCo]]

The verification was branch only, to be clear.