Closed Bug 11889 Opened 25 years ago Closed 25 years ago

[Dogfood] Ender does not display UTF-8 page correctly

Categories

(Core :: DOM: Editor, defect, P1)

Product:
Core
Component:
DOM: Editor
Platform:
x86
Windows NT
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: momoi, Assigned: ftang)

References

(
URL
)

Details

(Whiteboard: have fix on local build. Verified on Mac/Win/Linux. Jevering gave ok to check in. Need leaf's signal to check in) 

** Observed with 8/13/99 Win32 build **

** This bug was split from Bug 11763 which conflated 2 bugs into
   one and was confusing. Bug 11763 was made a duplicate of
   Bug 11094, but that bug was mainly about crashing, not display.
   So this bug has been created to deal with the UTF-8 display issue.

Originally this bug was a Mail bug but during the course of
discussion, it has become apparent that Editor has a problem
displaying this type of UTF-8 page. The sample page created by
RHP out of a reply/quoted message is at the above URL.

For a quick confirmation of this bug, view the above with Browser,
then, save it to a local disk. Then open it with Editor, you see that
the Japanese part is not displayed correctly.

Here are some comments by akkana on this problem from Bug 11763:

------- Additional Comments From akkana@netscape.com  08/13/99 14:30 -------
There's a style sheet issue here.  If you load this file in the browser, it
works.  If you load it in the editor, the editor starts but you see nothing in
the window.  But if you apply the style sheet "Editor Styles 1" (from
Format->Apply Style Sheet), everything looks fine on Linux, even the Japanese
text; on Mac, the Japanese text doesn't show up but everything else does.

=============
I tried 8/13/99 Win32 build, and found that applyging "Editor Styles 1"
did not bring out the Japanese display.

This type of web pages is used in forming a reply/quoted message
from a POP3/local Japanese message and if this does not work,
then we don't have a working reply function for Japanese
and other languages which use 8-bit characters.
QA Contact: sujay → momoi
Priority: P3 → P1
Assigning myself as QA contact.
akkana, we would like this to be fixed for M9 if at all
possible.
Summary: Ender does not display UTF-8 page correctly → [Dogfood] Ender does not display UTF-8 page correctly
I think this fits the definition of [Dogfood] for international
mail. Reply is such a vital part of everyday testing.
I'm not sure if someone already figured this out or not over the
weekend, I didn't see any mail about it, so I did some poking around to
try and figure out what was causing:

    http://bugzilla.mozilla.org/show_bug.cgi?id=11889

The problem here is that there is an assumption made that any html
document containing:

    <META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=utf-8">

is going to be parsed twice. There seems to be some code that gets
executed when running the browser, that forces the document to be parsed
twice, that isn't getting executed in Viewer or the Editor. I haven't
figured out what triggers this second parsing yet, perhaps
ftang@netscape.com can show us.

Since the document is only being parsed once in the Editor and Viewer,
the document is left in a non-rendering state because the
nsMetaCharsetObserver disables the document viewer rendering when the
parser encounters the body tag. This causes all screen refreshes to be
ignored. So when layout tries to enable refreshing, it does nothing.

In the browser, this is okay, because the second time the document is
parsed, a new document viewer is created, and there is no
nsMetaCharserObserver installed, so things proceed as normal. Like I
said before, this second parse does not happen in the Editor or Viewer
so we get stuck with the document viewer's render flag set to false.

For those of you debugging this problem, it is easy to see what is going
on by placing breakpoints in:

    nsViewManager :: DisableRefresh()
    nsViewManager::EnableRefresh()
    DocumentViewerImpl::SetEnableRendering()

Here's the stack trace that shows where the nsMetaCharsetObserver
disables rendering:

DocumentViewerImpl::SetEnableRendering(DocumentViewerImpl * const
0x011874b0, int 0) line 791
nsWebShell::SetRendering(nsWebShell * const 0x00fa91c8, int 0) line 2839
nsObserverBase::NotifyWebShell(nsObserverBase * const 0x00f5e638,
unsigned int 18379040, char * 0x0111a500, nsCharsetSource
kCharsetFromMetaTag) line 57 + 14 bytes
nsMetaCharsetObserver::Notify(nsMetaCharsetObserver * const 0x00f5e630,
unsigned int 18379040, unsigned int 4, unsigned short * * 0x0012fad0,
unsigned short * * 0x0012fb98) line 272 + 29 bytes
nsMetaCharsetObserver::Notify(nsMetaCharsetObserver * const 0x00f5e630,
unsigned int 18379040, unsigned short * 0x0012fab0, unsigned int 4,
unsigned short * * 0x0012fad0, unsigned short * * 0x0012fb98) line 155
nsObserverNotifier::operator()(void * 0x00f5e630) line 275 + 47 bytes
nsDeque::FirstThat(nsDequeFunctor & {...}) line 350 + 14 bytes
CNavDTD::WillHandleStartTag(CToken * 0x010415d0, nsHTMLTag
eHTMLTag_meta, nsCParserNode & {...}) line 1108
CNavDTD::HandleStartToken(CToken * 0x010415d0) line 1264 + 20 bytes
NavDispatchTokenHandler(CToken * 0x010415d0, nsIDTD * 0x01119a20) line
241 + 12 bytes
CTokenHandler::operator()(CToken * 0x010415d0, nsIDTD * 0x01119a20) line
80 + 14 bytes
CNavDTD::HandleToken(CNavDTD * const 0x01119a20, CToken * 0x010415d0,
nsIParser * 0x027a6910) line 731 + 18 bytes
CNavDTD::BuildModel(CNavDTD * const 0x01119a20, nsIParser * 0x027a6910,
nsITokenizer * 0x011198e0, nsITokenObserver * 0x00000000, nsIContentSink
* 0x011879b0) line 551 + 20 bytes
nsParser::BuildModel() line 941 + 34 bytes
nsParser::ResumeParse(nsIDTD * 0x00000000, int 0) line 886 + 11 bytes
nsParser::OnDataAvailable(nsParser * const 0x027a6914, nsIChannel *
0x011864a0, nsISupports * 0x00000000, nsIInputStream * 0x011869e0,
unsigned int 0, unsigned int 129) line 1168 + 19 bytes
nsDocumentBindInfo::OnDataAvailable(nsDocumentBindInfo * const
0x01186c70, nsIChannel * 0x011864a0, nsISupports * 0x00000000,
nsIInputStream * 0x011869e0, unsigned int 0, unsigned int 129) line 2057
+ 32 bytes
nsOnDataAvailableEvent::HandleEvent(nsOnDataAvailableEvent * const
0x01186be0) line 350
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x01186be4) line 149 + 12
bytes
PL_HandleEvent(PLEvent * 0x01186be4) line 509 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f5ec80) line 470 + 9 bytes
_md_EventReceiverProc(void * 0x005d0538, unsigned int 49333, unsigned
int 0, long 16116864) line 932 + 9 bytes
USER32! 77e71250()
The viewer problem have been traced down by harishd and me. The problem is
after nsObserverBase::NotifyWebShell call nsWebShell::SetRendering, it call
wss->ReloadDocument. But radha recently change the ReloadDocument method but
somehow, it seems mSHist is null in the  nsWebShell::ReloadDocument function
(line 2792) and therefore s is null (in line 2807), but we do not return
error to the caller in this case which make the process stop here.

What we should do is
1. find out why mSHist is null and fix it.
2. In the case of s is null, return error code to the caller
3. The caller should set the SetRendering(PR_TRUE) if ReloadDocument return
NS_SUCCEEDED(res)

1. is essential to make the viewer behavie correctly.
2. and 3 are just better error handling.Fix 2 and 3 along will make the
viewer display text with garbage, but at least, it will display something.

Harishd and Nisheeth, they will discuss this Meta charset issue in the Wed
Mozilla meeting. Could you attend ?

 57    if(NS_FAILED( rv = wss->SetRendering(PR_FALSE) ))
 58      goto done;
 59
 60    // XXX nisheeth, uncomment the following two line to see the reent
problem
 61
 62    // if(NS_FAILED(rv = wss->StopDocumentLoad()))
 63    //   goto done;
 64
 65    if(NS_FAILED(rv = wss->ReloadDocument(charset, source)))
 66      goto done;

2775 NS_IMETHODIMP
2776 nsWebShell::ReloadDocument(const char* aCharset,
2777                            nsCharsetSource aSource)
2778 {
2779
2780
2781   // XXX hack. kee the aCharset and aSource wait to pick it up
2782   mHintCharset = aCharset;
2783   mHintCharsetSource= aSource;
2784   nsString * s=nsnull;
2785
2786   /* The generic session history code that is in webshell
2787    * is now obsolete. Use nsISessionHistory instead
2788    */
2789 #ifdef OLD_HISTORY
2790   s = (nsString*) mHistory.ElementAt(mHistoryIndex);
2791 #else
2792   if (mSHist) {
2793      PRInt32  indix = 0;
2794      nsresult rv;
2795      rv = mSHist->getCurrentIndex(indix);
2796      if (NS_SUCCEEDED(rv)) {
2797         const PRUnichar * url=nsnull;
2798         rv = mSHist->GetURLForIndex(indix, &url);
2799         if (NS_SUCCEEDED(rv))
2800           s = new nsString(url);
2801      }
2802   }
2803
2804 #endif
2805
2806   nsresult rv = NS_OK;
2807   if(s) {
2808     char* url = s->ToNewCString();
2809     if(url) {
2810 #ifdef  OLD_HISTORY
2811       mHistoryIndex--;
2812 #endif  /* OLD_HISTORY */
2813       /* When using nsISessionHistory, the refresh callback should
2814        * probably call LoadURL with PR_FALSE as the third argument.
2815        * Not sure how well this method is being used.
2816        */
2817       rv =  this->RefreshURL((const char*)url,0,PR_FALSE);
2818       delete [] url;
2819     }
2820     delete s;
2821   }
2822
2823   return NS_OK;
2824 }
2825

More....

It turn out that SetSessionHistory only set by
xpfe/AppCores/src/nsBrowserAppCore.cpp
That is why viewer and Editor have a null mSHist.
Also, the current behavior in browser is some sort of borken because it require
hit the "Back" 3 times to really Back if the meta charset is not equal to the
defautl charset (for example, try http://home.netscape.com/ja). However, that is
a seperate bug and we should trace that problem in seperate bug report.

What should we do with ReloadDocument ? Basically, the ReloadDocument need to
find the URL of the currently page, reload the URL and make sure it does not
make the hsitory think it is a new url.

How can we find the current  URL in the case of Editor ?
Here's the 3 line fix to nsWebShell::ReloadDocument for the problem:


Index: nsWebShell.cpp
===================================================================
RCS file: /cvsroot/mozilla/webshell/src/nsWebShell.cpp,v
retrieving revision 1.212
diff -c -r1.212 nsWebShell.cpp
*** nsWebShell.cpp      1999/08/10 21:36:38     1.212
--- nsWebShell.cpp      1999/08/16 17:49:57
***************
*** 2800,2805 ****
--- 2800,2808 ----
            s = new nsString(url);
       }
    }
+   else {
+     s = new nsString(mURL);
+   }

  #endif


It fixes the problem for Viewer and the editor, unfortunately, it
exposes a known crash in the ~nsHTMLEditor() destructor which is not
related.

So should this fix go in for M9?
The crash in the editor looks like the old familiar double-load problem (the
document is being loaded twice, and the pres shell and other critical pieces are
changing out from under us, so we crash when we try to reference them).  With
Kin's patch to nsWebShell.cpp, nsEditorShell::OnStartDocumentLoad and
nsEditorShell::OnEndDocumentLoad are both called twice (and it's on the second
OnEndDocumentLoad that we crash).

We still need a solution to the double-loading problem.  Frank's patch to
intl/chardet/src/nsObserverBase.cpp (uncomment #define DONT_INFORM_WEBSHELL on
line 19) from bug 11094 seems to fix this -- the editor comes up with the
document correctly showing.

Both of these fixes should be checked in together (whether for M9 or M10).
Is bug 11094 the "known crash in the ~nsHTMLEditor() destructor"?
Will fixing 11889 make the 11094 worse?  Will anything that works now, start
to crash?  Or are we enabling additional functionality that may crash?

Also, bug 11094 is marked M9 critical, so we hope this will get fixed soon.
I have lots of changes in my local tree so I'm probably not a good test case,
but I am having major problems displaying mail messages because of this bug. So
if there is a way to get this working for M9, I think its pretty important.

- rhp
Target Milestone: M10
I'm marking this M10 until I hear otherwise.  If someone wants to convince
jevering that it needs to be in M9, and Frank is willing to check in the
nsObserverBase.cpp fix for double reloading, I have both fixes ready in my
tree.  If Frank thinks that his nsObserverBase.cpp fix isn't the right one and
we need to wait for rpotts to come up with a different solution for 11094, then
this bug will have to wait on that one.
To answer bobj's question, yes 11094 is the "known bug".

Fixing 11889 gets rid of the refresh problems we are having when loading
UTF8 problems, but it has the side effect of allowing the loading process to
continue, as it should, and get to the problem in bug #11094.
hum... I feel strange to akana's commente. I though the only problem is the
history problem. I try the 0806 build ( one day before radha's change) and try
to load a local copy of http://home.netscape.com/ja by using the open in editor.
And it work nicely without any problem (in my NT). I think the crash is caused
by some other
code changes, otherwise there are no reason 0806 build load the japanese
correctly without crashing. (It already do a reload in 0806 build)
The more I think about this, the more I can't understand how anyone is seeing
any mail messages being displayed with M9. Every body being displayed in mail
has the "<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=utf-8">"
line at the top so I would think this would break it all.

I have been having fits over the weekend with this problem so I'll add my $0.02
(again :-) that I think it is important that this gets in for M9.

- rhp
Depends on: 10335, 11094
Frank, without the change to nsObserverBase, I'm still seeing a double load, and
hence an editor crash because things are changing out from under us.

rpotts just marked 11094 a dup of 10335; I'm marking this bug dependant on both
of those.  I guess we need to wait and see what he does with that one.
Assignee: akkana → ftang
Reassigning to ftang -- it's not my decision which fix to the double-loading
problem gets checked in.
Target Milestone: M10 → M9
Look, I know what we are doing with mail and I can't (for the life of me)
understand how the M9 builds with this bug display any message bodies at all.
If we don't fix this, its a problem waiting to happen.

I don't care what the solution is and its not my place to offer an opinion on
that, but I will say that we need this fixed. This has been a known problem
bouncing around for weeks. I want to get this solved for M9 because I leave for
sabbatical in 4 days and I don't want someone else to waste the time I've spent
understanding what the problem is with respect to mail when I'm out.

- rhp
Status: NEW → ASSIGNED
There are three reasons cause this problem and I think I responsed for one [a
minor one] of them.
Problem 1: nsObserverBase::NotifyWebShell() does not call
nsWebShell:wss->StopDocumentLoad())) directly before calling ReloadDocument
62               // if(NS_FAILED(rv = wss->StopDocumentLoad()))
63               //   goto done;

I think this is a minor issue since ReloadDocument eventually will call the same
stuff itself, just a little bit later.  Calling StopDocumentLoad() before call
ReloadDocument won't make it too much better, although it will save some
performance.

Problem 2. Necko does not indicate ABORT or ERROR in the aStatus in
OnEndDocumentLoading
Problem 3. nsEditorShell::OnEndDocumentLoading does not check aStatus before it
call PrepareDocumentForEditing

There are no way for me to fix all three problems. I can fix the problem #1, but
that won't make a big differences here. The major problems are #2 and #3. Add
rpotts to the CC list.
Whiteboard: have fix on local build. Verified on Mac/Win/Linux. Jevering gave ok to check in. Need leaf's signal to check in
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
fixed and check in (both M9 branch and M10 tip)
You mentioned 3 problems and said that you can fix only one.
Were all 3 fixed or just 1? What can we expect in the way of results?

1. UTF-8 page can be displayed correctly.
2. POP3 mail's reply-forward/quote should now work?

Is item 2 reasonable expectation?
Status: RESOLVED → VERIFIED
** Checked with 8/24/99 Win32 M9 build **

I can display the UTF-8 pages with a meta tag in Editor.
I can also quote in reply/forward header & text in Latin 1, Japanese
and UTF-8 correctly.

Based on these finding, I'm going to mark this bug fixe verified.
You need to log in before you can comment on or make changes to this bug.