Closed Bug 264852 Opened 20 years ago Closed 20 years ago

View Source produces XML Parse Error in viewSource.xul

Categories

(Thunderbird :: Mail Window Front End, defect)

Product:
Thunderbird
Component:
Mail Window Front End
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Thunderbird0.9

People

(Reporter: mozbugs, Assigned: mscott)

References

Details

(Keywords: regression)

Attachments

(2 files)

fix the entity errors for thunderbird
4.25 KB, patch
jst
: superreview+
Details | Diff | Splinter Review
for historical purposes, same patch but resolved some cvs conflicts
4.64 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20041013 Firefox/0.10
Build Identifier: Thunderbird / 0.8 (20041018) (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3)

For any email message (pop, imap, other) when I attempt to use the "View Source"
Pulldown I get the following error:
XML Parsing Error: undefined entity
Location: chrome://global/content/viewSource.xul
Line Number 69, Column 5:
    <key id="key_find" key="&findOnCmd.commandkey;" command="cmd_find"
modifiers="accel" />\


Reproducible: Always
Steps to Reproduce:
1. Click on a message
2. Select "View Source"

Actual Results:  
The above error

Expected Results:  
It should have brought up a window containing the RFC-2822 message.
Same problem here on Windows 2000 Pro SP4 with the today's nightly (Thunderbird
/ 0.8 (20041018)).
Problem also exists on Linux.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.3) Gecko/20041017 Thunderbird/0.8
Mnenhy/0.6.0.104

I'm building Tb myself. Is it missing some extension?
Requesting to change Hardware/OS to All/All. Requesting Blocking.
Flags: blocking-aviary1.0?
Modified as requested.
OS: MacOS X → All
Hardware: Macintosh → All
I am also building my own.
cc'ing Benjamin. Maybe a locale re-org change related to view source's dtd files?
actually I don't see any DTD declaration in the tree for the missing entity in
question:

findOnCmd.commandkey

from:
http://lxr.mozilla.org/aviarybranch/source/toolkit/components/viewsource/content/viewSource.xul#109

I wonder why firefox doesn't get this missing DTD error too. It seems to be AWOL.

Blake, have there been any recent changes to view source for the new find
toolbar that would have caused this entity to go away or something?
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Okay. Shoot.

Scott, so originally what I did was create a separate findBar.dtd in
toolkit/locales, as you'd expect. But I'd completely forgotten about the l10n
freeze, and Benjamin (rightly) said it was too late for that. So instead, I was
forced to include browser.dtd in viewSource.xul, since that DTD file already
contains the right strings and stuff. Which won't work in TB, as I forgot. Can
anyone think of a clever way to fix this? I assume TB is under the same l10n
freeze as Firefox.
Oh, to answer your question: there definitely have been changes. The View Source
window didn't used to use the find bar, but the old dialog instead. I had a 1.0+
bug to fix this. The bright side is that in fixing it, I made the find bar
really easy for you to drop in to Thunderbird. But the issue we need to figure
out is the DTD, and that I couldn't add a new one so I had to reuse browser.dtd. 
Scott, assuming we can't just add the toolkit findBar.dtd (we should double
check with Benjamin), we could just ifdef it to only use the find bar in Firefox
and continue using the old find dialog in TB.

I guess my only regret there is that part of the benefit of this work was that
all this work was supposed to make it easy to slot the find bar in
anywhere...such as into TB itself. But if we can't add strings then we can't do
that, since the strings are only in browser.dtd right now.
actually we aren't under the same l10n freeze that you are so that may give us
some wiggle room.
blake, if it's easy to do, I'd lean towards ifdefing to use the old find dialog
within view source for now. Mostly because there isn't any qute artwork for the
find bar and I'd have to go scrambling for artwork to make it look right. Also,
we have a find in message feature already in the quick search bar so we wouldn't
be taking advantage of the new toolkit find in message bar in the main mail
window, so view source would be the only place you would see it for now.

So short term I'd like to go the idef old find dialog approach if we can.
Long term: possibly converge on using the new find bar.
Because this bug is blocking the 0.9 release I request to set the priority at
least to p2
Flags: blocking-aviary1.0? → blocking-aviary1.0+
make the findbar phoenix only
Keywords: regression
Target Milestone: --- → Thunderbird0.9
Attachment #163085 - Flags: superreview?(firefox)
Since you're talking about this, it looks like there's a window of opportunity
for fixing a missing feature.  It would be great if you could implement FAYT for
message bodies.  That, it would make a lot of sense and add towards FF and TB
coherence.
While I agree FAYT would be nice, I don't think adding it should be considered a
showstopper for aviary.  I suggest opening another bug as an RFE.

In reviewing the patch from Scott McGregor I see that he has done precisely what
he said he was going to do with the #ifdefs.  My question is whether someone
should now open up some sort of tracking bug to unroll this patch and reunify
the source?  That might also allow for the FAYT feature.

Thanks for your good work.
Ok, so I'm new at this.  I tried applying the patch and HUNK 2 failed in
viewSource.xul (see below- patch -p0 from toolkit/components/viewsource).  So I
made the change manually.  After recompiling and running I was able to view
source as before.  Should this pass review?  Pilot error on my part?

Here below is the failed patch hunk.  Note that this is on a very vanilla source
tree.



my-mac% cat viewSource.xul.rej 
***************
*** 73,83 ****
  
    <script type="application/x-javascript"
src="chrome://global/content/globalOverlay.js"/>
    <script type="application/x-javascript"
src="chrome://global/content/printUtils.js"/>
    <script type="application/x-javascript"
src="chrome://global/content/findBar.js"/>
    <script type="application/x-javascript"
src="chrome://global/content/viewSource.js"/>
    <script type="application/x-javascript"
src="chrome://global/content/viewZoomOverlay.js"/>
- 
    <stringbundle id="bundle_findBar"
src="chrome://browser/locale/browser.properties"/>
    <stringbundle id="viewSourceBundle"
src="chrome://global/locale/viewSource.properties"/>
  
    <command id="cmd_savePage" disabled="true" oncommand="ViewSourceSavePage();"/>
--- 75,90 ----
  
    <script type="application/x-javascript"
src="chrome://global/content/globalOverlay.js"/>
    <script type="application/x-javascript"
src="chrome://global/content/printUtils.js"/>
+ #ifdef MOZ_PHOENIX
    <script type="application/x-javascript"
src="chrome://global/content/findBar.js"/>
+ #else
+   <script type="application/x-javascript"
src="chrome://global/content/findUtils.js"/>
+ #endif
    <script type="application/x-javascript"
src="chrome://global/content/viewSource.js"/>
    <script type="application/x-javascript"
src="chrome://global/content/viewZoomOverlay.js"/>
+ #ifdef MOZ_PHOENIX
    <stringbundle id="bundle_findBar"
src="chrome://browser/locale/browser.properties"/>
+ #endif
    <stringbundle id="viewSourceBundle"
src="chrome://global/locale/viewSource.properties"/>
  
    <command id="cmd_savePage" disabled="true" oncommand="ViewSourceSavePage();"/>
Eliot, the patch doesn't apply cleanly anymore because some lines in
viewSource.xul, which the patch wants to modify, were changed in the meantime.
So you need to apply it by hand. Nothing to worry about.
Ok, then.  I stand corrected.  The fix itself works flawlessly.  can we get a
superreview?
Comment on attachment 163085 [details] [diff] [review]
fix the entity errors for thunderbird

sr=jst
Attachment #163085 - Flags: superreview?(firefox) → superreview+
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
There's a typo in one line of the patch-- in the ifdef prior to cmd_find in
viewSource.xul, MOZ_PHOENIX is misspelled MOZ_PHONEIX.
Thanks Cody! Shame on me. I just fixed the typo but today's nightly will
probably have problems for firefox view source since it's already out. 
*** Bug 266535 has been marked as a duplicate of this bug. ***
*** Bug 266664 has been marked as a duplicate of this bug. ***
*** Bug 267115 has been marked as a duplicate of this bug. ***
vrfy'd fixed with recent tbird trunk builds, eg, 2005060606-trunk on Mac OS X
10.4.1.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: