xul error when clicking on link that results in download of file from webapp

RESOLVED FIXED in Firefox 27

Status

defect
P2
normal
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: dietrich, Assigned: marco)

Tracking

unspecified
Firefox 27
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Posted image screenshot
screenshot attached.
Hmm, looks like unknownContentType.xul didn't load unknownContentType.dtd, which is strange, since both should be packaged with Toolkit files, which should all be available to the desktop webapp runtime.  They are, however, packaged into different JAR archives using different manifests (toolkit/mozapps/downloads/jar.mn and toolkit/locales/jar.mn, respectively), so perhaps the localization archive isn't being loaded for some reason.

glandium did some runtime localization work over in bug 762864.  I wonder if he would know why this is happening.
Yes, the code there looks fine - correct URL, etc. I can load the chrome URL to the dtd in the browser, and shows the right file.
Glandium says nope, doesn't know.
Duplicate of this bug: 901205
WARNING: Failed to open external DTD: publicId "" systemId "chrome://branding/locale/brand.dtd" base "chrome://mozapps/content/downloads/unknownContentType.xul" URL "chrome://branding/locale/brand.dtd"

So it's brand.dtd (and brand.properties) that aren't packaged for the runtime.
None of the entities declared in brand.dtd are used in unknownContentType.xul. Sounds like you can just remove the reference.
While doing this, it might be worth checking other files under toolkit/ that include brand.dtd (there are a bunch of them) to see if they actually use it.
Assignee: nobody → mcastelluccio
Status: NEW → ASSIGNED
Posted patch Patch (obsolete) — Splinter Review
Attachment #786733 - Flags: review?(mh+mozilla)
Comment on attachment 786733 [details] [diff] [review]
Patch

I'm not a toolkit peer.

That being said, the settingsChangePreferences and settingsChangeOptions labels are interesting, in that they may not match anything real outside Firefox (and in fact, don't in webapps). It seems to me this part of the UI should be browser-specific instead of being in toolkit.
Attachment #786733 - Flags: review?(mh+mozilla) → review?(dtownsend+bugmail)
Posted patch Patch v2 (obsolete) — Splinter Review
Attachment #786733 - Attachment is obsolete: true
Attachment #786733 - Flags: review?(dtownsend+bugmail)
Attachment #787208 - Flags: review?(dtownsend+bugmail)
Comment on attachment 787208 [details] [diff] [review]
Patch v2

Review of attachment 787208 [details] [diff] [review]:
-----------------------------------------------------------------

You seem to be missing part of this patch (settingsChange.properties).

::: toolkit/locales/en-US/chrome/mozapps/downloads/unknownContentType.dtd
@@ +3,5 @@
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
>  <!ENTITY  intro2.label                "You have chosen to open:">
>  <!ENTITY  from.label                  "from:">
> +<!ENTITY  actionQuestion.label2       "What would you like to do with this file?">

Can you get someone from UX to agree to this change?

::: toolkit/mozapps/downloads/nsHelperAppDlg.js
@@ +474,5 @@
>        openHandler.parentNode.removeChild(openHandler);
>        var openHandlerBox = this.dialogElement("openHandlerBox");
>        openHandlerBox.appendChild(openHandler);
> +
> +      if (Services.appinfo.ID != "webapprt@mozilla.org") {

I'd rather not hardcode here. Does attempting to load the bundle throw an exception?
Attachment #787208 - Flags: review?(dtownsend+bugmail) → review-
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
I hope I not breaking any conventions by posting this, but it may help, it certainly now allows me to get the expected behaviour until a fix is published. 

Taken from: https://developer.mozilla.org/en-US/docs/XULRunner_tips#Using_Firefox_to_run_XULRunner_applications

Whilst the error code stated below is different, with those preferences added the expected action occurs when clicking file download links from a webmail webapp.

To use the unknown-content-type and file-downloads dialogs from a <browser> element, you need to add the following prefs:

pref("browser.download.useDownloadDir", true);
pref("browser.download.folderList", 0);
pref("browser.download.manager.showAlertOnComplete", true);
pref("browser.download.manager.showAlertInterval", 2000);
pref("browser.download.manager.retention", 2);
pref("browser.download.manager.showWhenStarting", true);
pref("browser.download.manager.useWindow", true);
pref("browser.download.manager.closeWhenDone", true);
pref("browser.download.manager.openDelay", 0);
pref("browser.download.manager.focusWhenStarting", false);
pref("browser.download.manager.flashCount", 2);
//
pref("alerts.slideIncrement", 1);
pref("alerts.slideIncrementTime", 10);
pref("alerts.totalOpenTime", 4000);
pref("alerts.height", 50);

If you are missing preferences that a dialog requires, you will get the following errors:

Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]

Error: dialog has no properties
Source File: chrome://mozapps/content/downloads/u...ontentType.xul
Line: 1
(In reply to Andy Poole from comment #12)
> Whilst the error code stated below is different, with those preferences
> added the expected action occurs when clicking file download links from a
> webmail webapp.

The problem here was in the download dialog, I think with those preferences you're bypassing it and you're directly downloading the files in the default downloads directory.
I don't know if this is the expected behavior in an app, because while in Firefox users can change settings, in the Webapp Runtime they can't (so they'll be forced to always use the downloads directory).
Any update please.

Kind Regards, Aarif
Posted patch Patch v3 (obsolete) — Splinter Review
This creates a webapp specific branding. We'll later decide where we want Firefox's branding.
Attachment #787208 - Attachment is obsolete: true
Attachment #814240 - Flags: review?(dtownsend+bugmail)
Comment on attachment 814240 [details] [diff] [review]
Patch v3

Review of attachment 814240 [details] [diff] [review]:
-----------------------------------------------------------------

::: webapprt/Startup.jsm
@@ +69,5 @@
> +vendorShortName=' + developer;
> +
> +  yield writeFile(OS.Path.join(PROFILE_DIR, "brand.properties"),
> +                  brandPropertiesContent);
> +}

Everything looks fine here but I have a comment about these functions. It feels a little better to me to wrap them in a Task.spawn call so they just return a promise, this makes them easier to use from other contexts. If that's not the norm for webapprt code then that's fine I'm not set on it. I have started a thread on dev-platform to discuss this pattern.
Attachment #814240 - Flags: review?(dtownsend+bugmail) → review+
(In reply to Dave Townsend (:Mossop) from comment #16)
> Comment on attachment 814240 [details] [diff] [review]
> Patch v3
> 
> Review of attachment 814240 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: webapprt/Startup.jsm
> @@ +69,5 @@
> > +vendorShortName=' + developer;
> > +
> > +  yield writeFile(OS.Path.join(PROFILE_DIR, "brand.properties"),
> > +                  brandPropertiesContent);
> > +}
> 
> Everything looks fine here but I have a comment about these functions. It
> feels a little better to me to wrap them in a Task.spawn call so they just
> return a promise, this makes them easier to use from other contexts. If
> that's not the norm for webapprt code then that's fine I'm not set on it. I
> have started a thread on dev-platform to discuss this pattern.

I usually prefer this style, because it makes the code a little more readable (it avoids indentation).
It also makes the function impossible to use outside of a Task, so people in future will not be able to just call it and forget that it's asynchronous.
Anyway I'm fine with changing it until we decide in dev-platform what to do in general.
Posted patch Patch v3Splinter Review
Carrying r+.
Attachment #814240 - Attachment is obsolete: true
Attachment #814567 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d8109ff79880
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
Hello,

Does this mean that this fix will be work on FireFox 27. Currently we are using FireFox 24 - it isstill giving the same error.

XML Parsing Error: undefined entity
Location: chrome://mozapps/content/downloads/unknownContentType.xul
Line Number 30, Column 18:    <description>&intro2.label;</description>
-----------------^

Please confirm.

Kind Regards, Aarif
Hello Ryan,

Sorry, I can't undersatnd your reply. Can you please clarify - if the fix should work in FireFox 24 or it will be released with FireFox 27.

Kind Regards, Aarif
It will be released in Firefox 27.
Well, I was the author of the dublicate bug (see here: https://bugzilla.mozilla.org/show_bug.cgi?id=901205) and for me there is a new problem now.
With Firefox 27 there is a dialog box, as expected. But every downloaded file is empty. I've tested this with some .pdf and .csv files. Adobe Reader (11.0.6) says the .pdf file is broken. LibreOffice (4.2.0) opens the .csv file, but there is nothing in it (empty spreadsheet). It is the same with Microsoft Office 2013.
Please note: The same files, downloaded in normal browser (Firefox 27.0) work just fine. I can open and view all of them (.pdf's and .csv's).
Actually this new bug is described here:
https://bugzilla.mozilla.org/show_bug.cgi?id=965414
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.