174197 - font error when link bookmarked

Status: VERIFIED WORKSFORME

(Core :: Internationalization, defect)

Description

[pontus enhager]

the character encoding swithces when site icon dragged onto personal bar for
e.g. european fonts

Recipy:

1. Go to http://www.alternativet.nu
2. Check that the swedish characters å,ä,ö exists in the text
3. Bookmark the page as described
4. Reload
5. Swedish characters gone

i have seen this at more sites but i cannot recall their names for the moment

Comment 1

[Henrik Lynggaard Hansen]

I'm confirming this bug for now. This is what appear to be happening.


1. At first load the character coding  defaults to Western ISO-8859-1.
2. The user bookmarks and reloads the page
3. Now the character encoding defaults to UTF-8, making the special characters 
appear as ? signs

Additional information:

* Manually switching the character coding back to Western ISO-8859-1 works, but 
it is lost on the next reload (se below)

* It seems to be more connected with "Reload" than bookmarking. After having 
played a little with it:

  - Reloading seems to switch between the two encoding, thus having every 
second load be in UTF-8 and the rest in Western ISO, so basicly the site works 
half the time.
  - Loading new pages by following links does not switch encoding.

*  my tests are done with: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; 
rv:1.2b) Gecko/20021007

Comment 2

[Jean-Pierre Melkonian]

For me the enconding is windows-1252 (Mac OS 91. Build 2002100911). I have set
Auto detect Universal. 
It works fine.
I don't see any language meta tags.

At load, I have also the folowing JS Error:
Error: doc.layers has no properties
Source File: http://www.alternativet.nu/dhtmllib.js
Line: 373

Comment 3

[Christopher Hoess (gone)]

->Internationalization

Comment 4

[Simon Montagu :smontagu]

This happens when dragging and dropping from the URL bar to the Personal
toolbar, because navigatorDD.js passes to the bookmark service the charset of
the document where the drag originates, which in this case is a XUL document in
the chrome with charset UTF-8.

http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigatorDD.js#344

Comment 5

[Roy Yokoyama]

nhotta: can we do anything about this?

Comment 6

[Simon Montagu :smontagu]

*** Bug 177270 has been marked as a duplicate of this bug. ***

Comment 7

[Yuying Long]

Can reproduce it on windows latest trunk build, change platform to All.

I think it's a bad bug, nominate as nsbeta1.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Raising severity to major per duplicate bug.

I assume this is a regression.  Any ideas when it started?  It seems like an
ugly bug to have in 1.2.

Comment 9

[pontus enhager]

the bug has been present for Linux for very long (i would _guess_ pre 1.0)
i simply didn't  find the reason for the behaviour the first time

Comment 10

[José Jeria]

Attachment: Testcase from dupe.

Steps to reproduce:
 1. Load testcase.
 2. Drag the icon in the URL bar to the personal toolbar.
 3. Hit the back button.
 4. Click on the icon in the personal toolbar just created.

Comment 11

[Niklas Dougherty]

This seems to be a PC only bug. It doesn't happen on Mac OS X, and comment 2
suggests it doesn't happen in legacy Mac OS 9 either.

Comment 12

[Jean-Pierre Melkonian]

The TestCase works fine On Mac OS 9.1 Build 2002110808.
No problems

Comment 13

[Frank Tang]

I can reproduce this on window 01142003 build. It looks very bad. It show wrong
and right every other times.

Comment 14

[Samir Gehani]

i18n triage team: nsbeta1+/adt2

Comment 15

[Frank Tang]

assign

Comment 16

[Frank Tang]

I update my local build and I can no longer reproduce this problem. I think the
problem is fixed. mark it as wfm.
please reopen if you can still reproduce this.

Comment 17

[José Jeria]

Did you do the steps in comment 10? I still see the bug on Windows 2000, build
2003021208.

Comment 18

[José Jeria]

Attachment: Screenshot

Only thing that changed is that before i got the ?, now i get this black
squares instead.

Comment 19

[Asa Dotzler [:asa]]

both testcases work for me with today's linux build on RH8 but do not work on
today's windows build on winXP (sp1) or Mac OS X (10.2.3). The titlebar and the
content from the testcase in comment 10 are both broken. 

Comment 20

[Kevin McCluskey (gone)]

Simon: Can you take a look at this one? Its blocking Moz 1.3. Thanks!

reassigning to smontagu.

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(I think I was probably being a bit overzealous when I marked this as a blocker,
although a fix would be nice.  I don't think we'd really hold for it if it came
down to that.)

Comment 22

[Simon Montagu :smontagu]

Attachment: Patch

This patch is based on the idea in comment 4 and prevents the bug (using the
test procedure in comment 10).

However, I smell some bogosity:

(a) When I debug, aDragSession.sourceDocument always seems to be null except
when the drag originates in chrome.
(b) Does the current algorithm make sense? Why is the charset of the linking
document considered to have anything to do with the linked document?

Comment 23

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The only case I can see where you might want to remember the encoding is where
the drag originates from the URL bar, but in that case you'd want to remember
the encoding of the document being displayed, not the encoding of the XUL document.

If we can't detect that case correctly, then why not just make it null all the time?

Comment 24

[Simon Montagu :smontagu]

I wonder if we can search for the charset in global history, as we currently do
for the title at
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigatorDD.js#358

Comment 25

[Simon Montagu :smontagu]

Attachment: Patch v.2

This way of testing whether we are bookmarking the current document seems to
work...

Comment 26

[Simon Montagu :smontagu]

Attachment: Patch for 1.3 branch

... but perhaps it would be safer just to go with setting the charset to null
on the branch, and try to find out how to set it correctly for 1.4.

Comment 27

[Christopher Blizzard (:blizzard)]

Comment on attachment 115455 [details] [diff] [review]
Patch for 1.3 branch

sr=blizzard

Comment 28

[Simon Montagu :smontagu]

Comment on attachment 115455 [details] [diff] [review]
Patch for 1.3 branch

Requesting approval for 1.3 final.

As far as I can tell (see comment 22), the current code will never produce a
correct result except by coincidence and will often produce an incorrect result
as in this bug. This branch-only patch prevents the incorrect result by never
setting a charset in a bookmark created by dragging a link. I will leave the
bug open for investigating ways to set a correct charset at least some of the
time, but I don't want to hold up the branch for that.

Comment 29

[Simon Montagu :smontagu]

The fix is checked in to the 1.3 branch. Since there is no fixed1.3 keyword and
this is a blocker, should I close the bug so it disappears from the radar, and
open a new bug for the trunk?

Comment 30

[Yuying Long]

Verified fixed on 1.3 mozilla branch build, but still exists in 02-27 trunk build.

Comment 31

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Any progress on fixing this for the trunk?

Comment 32

[Simon Montagu :smontagu]

I'd like to extend attachment 115450 [details] [diff] [review] as described in comment 24 and then seek
reviews for the trunk, but I need to do some research first on how to do that,
or whether it is even possible.

Comment 33

[Simon Montagu :smontagu]

No, global history doesn't seem to know anything about charsets. My current
design for a fix is as follows:

If the URI being bookmarked is the current document, use the current charset.
Otherwise, if there is an existing bookmark for the URI, copy its charset.
Otherwise make the charset null.

Targetting to 1.4b, to keep it behind stuff that I want to get into 1.4a.

Comment 34

[José Jeria]

I dont know if it matters, but this was not completely fixed in Mozilla 1.3. I
still sometimes get the � instead of swedish characthers. Though the testcase in
this bug works fine.

Comment 35

[Simon Montagu :smontagu]

Attachment: Patch for trunk

It seems that in the new bookmarks world, no extra code is required to get the
charset from the current (focused) document, so this patch should be
sufficient.

Comment 36

[jag (Peter Annema)]

Comment on attachment 121216 [details] [diff] [review]
Patch for trunk

sr=jag

Comment 37

[Simon Montagu :smontagu]

Patch checked in.

Comment 38

[Yuying Long]

Verified fixed on 04-22 trunk build.

Comment 39

[José Jeria]

Seing this again with build 2003051504, Windows 2000:

To reproduce do this:
1. Save "Testcase from dupe." locally. 
2. Load the page, drag the link to the toolbar
3. Press the link, you will see the error

Also, reloading the page fixes the problem, to reproduce, remove the link from
the toolbar, restart mozilla and follow the steps above.

I can attach a screenshot if necessary to show that this happends with a new build.

Comment 40

[Yuying Long]

I can reproduce the problem by following the steps in previous comment on 05-16
trunk build / WinXP and Win2000:
When I load the bookmark of the test case in personal toolbar from saved LOCAL file.

However, when I bookmark the test case directorily from attachment, then I won't
see it.  The current problem is a little different from the orginal bug, because
it was seen even load the bookmark that directory drag from html page before. 
Maybe we should open a new bug for the current problem and close this one. 

Comment 41

[José Jeria]

This is not only when the file is saved locally, I have a swedish site 
bookmarked and I see this very often. Maybe because its cached?

Comment 42

[Simon Montagu :smontagu]

If you created the bookmark before this was fixed, I think you will go on seeing
the problem. Try deleting the bookmark and creating a new one.

Comment 43

[José Jeria]

Yes, you are right, 
Should I open a new bug for comment 39 or leave this one reopened?

Comment 44

[Simon Montagu :smontagu]

I think comment 39 should be filed as a new bug. Marking this WORKSFORME.

Comment 45

[Yuying Long]

Mark as verified for this bug.  Please open a new bug for the problem in comment
39 with clear steps, thanks!

Comment 46

[José Jeria]

Bug 207338 was filed for was is mentioned in comment 39