Closed Bug 846251 Opened 12 years ago Closed 11 years ago

XML parsing error in blocklist.xul

Categories

(Firefox Graveyard :: Web Apps, defect, P1)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 27

People

(Reporter: adora, Assigned: marco)

References

Details

(Keywords: regression, Whiteboard: [apps watch list] DesktopWebRT2)

Attachments

(1 file, 4 obsolete files)

Periodically, I get the following error message while running apps on Firefox Nightly on desktop: XML Parsing Error: undefined entity Location: chrome://mozapps/content/extensions/blocklist.xul Line Number 18, Column 1:<dialog windowtype="Addons:Blocklist" title="&blocklist.title;" align="stretch" Screenshot: http://adora.io/screens/blocklist_error_16DF6183.png Right now I'm seeing it with an app in the review queue called DatKey, but there have been others. https://marketplace.firefox.com/reviewers/apps/review/datkey
Whiteboard: [apps watch list]
Probably a regression with the blocklist integration with desktop apps. Sadly, the desktop implementation has collected a bunch of cruft as a result of advancing b2g work. And we have little resources allocated to it, so I don't have an ETA of when something like this would be fixed.
Keywords: regression
As long as the error doesn't mean I'm getting teh hax0rd, then this isn't a priority for me now.
Lisa please check to see if this bug is still present.
Assignee: nobody → lbrewster
Yep, still seen in Nightly 2013-06-05.
Yep, still seen in Nightly 2013-06-05.
Assignee: lbrewster → nobody
Could I have access to that application? Or, do you have another test case publicly accessible?
Flags: needinfo?(adora)
It happens randomly while using many desktop apps. I don't have STR, best advice I can give is to try a few apps from Marketplace and see if it comes up.
Flags: needinfo?(adora)
Priority: -- → P1
Same problem as bug 851217, I guess we can simply modify the strings to avoid using brand.dtd. The guilty strings are: "&brandShortName; has determined that the following add-ons are known to cause stability or security problems:" "Restart &brandShortName;"
Attached patch fix_blocklistxul_webapprt (obsolete) (deleted) — Splinter Review
Attachment #786736 - Flags: review?(mark.finkle)
We might be able to simply avoid using brand-specific strings in some cases (or switch to strings generated dynamically from .properties files), but in this case the dialog in question shouldn't be showing up in the first place, since addons aren't enabled in the runtime.
(In reply to Myk Melez [:myk] [@mykmelez] from comment #10) > We might be able to simply avoid using brand-specific strings in some cases > (or switch to strings generated dynamically from .properties files), but in > this case the dialog in question shouldn't be showing up in the first place, > since addons aren't enabled in the runtime. We're using the blocklist also for plugins.
(In reply to Marco Castelluccio [:marco] from comment #11) > We're using the blocklist also for plugins. (That in our case is only Flash)
(In reply to Marco Castelluccio [:marco] from comment #11) > We're using the blocklist also for plugins. Ah, sorry, I missed that in this case the word "add-ons" is being used in its more general sense rather than as a synonym for "extensions"! So, in some cases, where Firefox uses its own name, we'll want to use the app's name (like we do in the titlebar). And in other cases we might want to drop the name. But in this case, I think we do want to identify Firefox as the agent of the blockage, so the user understands who/what is responsible for blocking the plugin.
Comment on attachment 786736 [details] [diff] [review] fix_blocklistxul_webapprt I see what you mean, some users could be confused and think that "Restart" means "Reboot your computer".
Attachment #786736 - Attachment is obsolete: true
Attachment #786736 - Flags: review?(mark.finkle)
Attached patch Patch (obsolete) (deleted) — Splinter Review
With this patch the blocklist dialog will use the webapp name, in the webapp runtime, or the branding name. As I was there, I've also removed a hack introduced to avoid creating new strings during a string freeze.
Assignee: nobody → mcastelluccio
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) (deleted) — Splinter Review
Attachment #787163 - Attachment is obsolete: true
Attachment #787183 - Flags: review?(mark.finkle)
Attached patch Patch (obsolete) (deleted) — Splinter Review
Forgot to qref...
Attachment #787183 - Attachment is obsolete: true
Attachment #787183 - Flags: review?(mark.finkle)
Attachment #787184 - Flags: review?(mark.finkle)
Comment on attachment 787184 [details] [diff] [review] Patch The webrt changes look OK to me, but the toolkit changes are out of my league. Passing to Gavin to find a reviewer.
Attachment #787184 - Flags: review?(mark.finkle)
Attachment #787184 - Flags: review?(gavin.sharp)
Attachment #787184 - Flags: review+
Comment on attachment 787184 [details] [diff] [review] Patch I'm waiting for Mossop ideas about how to handle this in general (he wasn't happy with the Services.appinfo.ID check).
Attachment #787184 - Flags: review?(gavin.sharp)
We've thought that it's better if the users identify Firefox as the blocking agent.
Attached patch Patch (deleted) — Splinter Review
Attachment #787184 - Attachment is obsolete: true
Attachment #791572 - Flags: feedback?(l10n)
Note: Axel will be AFK for most of the next 10 days, and my knowledge of this area is far too limited to give meaningful feedback (for example I can't understand the reason behind the change in JarMaker.py). I'll leave the f? open for Axel, just wanted to let you know in case the patch is urgent.
Comment on attachment 791572 [details] [diff] [review] Patch Review of attachment 791572 [details] [diff] [review]: ----------------------------------------------------------------- I don't think this is the right way of doing. My problem is that I also don't exactly know what we're doing. Is this brand.dtd consistently not being part of the right package? How is that package packaged? Can we do something like https://mxr.mozilla.org/mozilla-central/source/browser/locales/Makefile.in#152 but with a different xpi-name?
Attachment #791572 - Flags: feedback?(l10n) → feedback-
(In reply to Axel Hecht [:Pike] from comment #23) > I don't think this is the right way of doing. My problem is that I also > don't exactly know what we're doing. > > Is this brand.dtd consistently not being part of the right package? How is > that package packaged? Can we do something like > https://mxr.mozilla.org/mozilla-central/source/browser/locales/Makefile. > in#152 > but with a different xpi-name? The webapp runtime and Firefox have two separate packages. The problem is that we want the Firefox's brand.dtd and brand.properties to be packaged also for the webapp runtime. The webapp runtime package is built using these two jar manifests: http://mxr.mozilla.org/mozilla-central/source/webapprt/jar.mn and http://mxr.mozilla.org/mozilla-central/source/webapprt/locales/jar.mn So, I wanted to include the brand.dtd and brand.properties in the webapp runtime package, but the localized versions of those files can be in an absolute directory outside the Mozilla source code. This is why I had to modify JarMaker.py.
Seems the webapprt magic includes at least the http://mxr.mozilla.org/mozilla-central/source/webapprt/defs.mk with DIST_SUBDIR=webapprt I'm pretty sure we don't need or should modify jarmaker, but build browser branding with the right flags for webapprt. I.e., do that twice. Gregory, can you guide us here, I'm traveling and don't have much time for trial and error (which is what I'd do ;-) )
Flags: needinfo?(gps)
I don't have an answer on the top of my head. One of the two Mike's should hopefully have insight.
Flags: needinfo?(mshal)
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(gps)
I'm not too thrilled by the webapp runtime getting the firefox branding. In fact, I wanted to move it from app to platform, making it part of xulrunner, where such branding is not available. Can't we get the webapp runtime to create a brand.dtd (virtually in memory or physically on disk) for each installed app instead?
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #27) > I'm not too thrilled by the webapp runtime getting the firefox branding. In > fact, I wanted to move it from app to platform, making it part of xulrunner, > where such branding is not available. > > Can't we get the webapp runtime to create a brand.dtd (virtually in memory > or physically on disk) for each installed app instead? We need to use the Firefox branding in the blocklist dialog, because we want the user to identify that is Firefox that is blocking a plugin. If we don't want to package the Firefox branding in the webapp runtime package, we could (I don't know yet how) read the Firefox branding file from the Firefox package when it is requested. This solution would probably be less easy. Note that we can't store the branding on disk during installation, because the webapp could be executed by different versions of Firefox (Aurora, Nightly, that have their own branding). e.g. it could be installed through Nightly and later executed through Aurora, we'll want the Aurora branding in this case.
I was implying to use the app name for the app-specific branding. That being said, I also don't see user identifying Firefox as blocking the plugin a good thing, because they'll get that prompt from Firefox itself saying it's Firefox (obviously), and then from every webapp, saying it's Firefox, at which point they'd likely say "Can't Firefox just get its act together and stop annoying me about that plugin already?".
(In reply to Mike Hommey [:glandium] from comment #29) > That being said, I also don't see user identifying Firefox as blocking the > plugin a good thing, because they'll get that prompt from Firefox itself > saying it's Firefox (obviously), and then from every webapp, saying it's > Firefox, at which point they'd likely say "Can't Firefox just get its act > together and stop annoying me about that plugin already?". True, but the "bad thing" here is that Firefox doesn't yet support sharing this information across apps, and pretending the apps are to blame for that is even worse. Firefox is in fact the agent of the dialogs and the software that doesn't have its act together, and until we fix that problem, it should take the blame.
I don't think I can provide any additional information here.
Flags: needinfo?(mshal)
Whiteboard: [apps watch list] → [apps watch list] DesktopWebRT2
This was fixed by bug 851217. I've opened bug 930737 to discuss about when we want to use Firefox's branding and when the webapp branding.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 851217
Target Milestone: --- → Firefox 27
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: