Henrik Skupin [:whimboo][⌚️UTC+2] Reporter
	Description • 19 years ago

With the pre-final patch on bug 280190 we recognized a problem for embedded
applications. All of them don't handle their own branding. For example Camino
still holds chrome://global/locale/brand.dtd but the referenced realBrandDTD
isn't packaged. Due to this reason using brand.dtd causes a XML parser error.

A realBrandDTD should be packaged for each of the embedded apps. After it's done
we could insert the branding stuff (attachment 189994 [details] [diff] [review]) into netError.xhtml again.

	Mike Pinkerton (not reading bugmail)
	Comment 1 • 19 years ago

my guess is conrad shouldn't be the owner.

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 3 • 19 years ago

Comment on attachment 190996 [details] [diff] [review]
Core part for using brandFullName in netError.xhtml

r=biesi on the condition that this doesn't land until this works in camino

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 4 • 19 years ago

Comment on attachment 190996 [details] [diff] [review]
Core part for using brandFullName in netError.xhtml

actually... bsmedberg, is this still the right way to include brand.dtd, or
should new code use the /branding/ url directly?

	Benjamin Smedberg
	Comment 5 • 19 years ago

I'm internally conflicted here. If we're treating the embedding API as frozen,
then we should not add a new requirement for embedders to provide a branding
package. Ugh.

The ideal situation would be to have stringbundles which could be used by
untrusted script, but that's something that needs 1.9 love.

Darin, opinions? We could theoretically ship a branding package with some sort
of generic "Embedding Application" brand.dtd and let everyone override it, but
that still kinda sucks.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 6 • 19 years ago

Right now this is blocking bug 301208 (the text of the error messages) which is
hoping to be able to use the product name dynamically. Any chance of it making
it for 1.5? Or at least some solution making it for 1.5? If not, I'll need to
either hardcode the product name for Firefox (which, I'm guessing, isn't a
popular choice!) or replace it everywhere with "the browser" (which makes the
messages a touch more obscure)

	Darin Fisher
	Comment 7 • 19 years ago

So long as embedders can hack around this problem, this solution should be fine
for Gecko 1.8.  Anyways, haven't we been saying that embedders would likely
replace netError.xhtml with their own error pages?  At least, isn't that what
epiphany does?  I agree that we should come up with something better for Gecko 1.9.

	Benjamin Smedberg
	Comment 8 • 19 years ago

Comment on attachment 190996 [details] [diff] [review]
Core part for using brandFullName in netError.xhtml

I'd say at this point our embedders should not have to provide a branding
package. We either need to ship a default branding package and override it for
the app, or come up with a better solution (perhaps have Firefox override the
netError.dtd itself?)

	Nickolay_Ponomarev
	Comment 9 • 19 years ago

Other parts of toolkit too rely on the branding package (e.g. bug 313133). Given that, it might make more sense to provide a default branding package than to try to change the toolkit not to depend on it.

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 19 years ago

So... it looks like this missed 1.8.  What's out 1.8 story for embeddors at this point?

	Brendan Eich [:brendan]
	Comment 11 • 19 years ago

(In reply to comment #10)
> So... it looks like this missed 1.8.  What's out 1.8 story for embeddors at
> this point?

Not sure, but we can get it together as a delta to 1.8, tag that tree, release it as XULRunner 1.8 (is that the version?  bsmedberg knows all).  Isn't that our embedding plan of record?

/be

	Benjamin Smedberg
	Comment 12 • 19 years ago

I thought we fixed this by making the docshell netError.xhtml generic and overriding it from Firefox...

	Boris Zbarsky [:bzbarsky]
	Comment 13 • 19 years ago

If we did, great.  Please resolve this as appropriate.

	Benjamin Smedberg
	Comment 14 • 19 years ago

Since the "pre-final patch on bug 280190" was never checked in, this isn't a problem in 1.8. This was fixed for Firefox specifically in bug 305998.

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 15 • 19 years ago

reopening, that's just a workaround and we should fix this.

	Darin Fisher
	Comment 16 • 18 years ago

Not going to block 1.8.1 for this bug.

	Smokey Ardisson (offline for a while; not following bugs - do not email)
	Comment 17 • 18 years ago

How difficult would it be to ship a generic branding package as Benjamin suggested in comment 5?  

pipnss strings are *already referencing branded strings on the 1.8.0 branch* as well as 1.8.1 and trunk (causing ugliness in security warnings, bug 343837) and more and more shared code seems to want to reference the branding strings but is stymied by this bug (bug 301208, bug 339720)

	Benjamin Smedberg
	Comment 18 • 18 years ago

A default branding package is very hard to do properly. It would require code changes in the chrome registry in order to act as a "backup" properly. We've talked about adding "backup" code to the chrome registry in general, for theming and other purposes, but no acceptable proposals have been put forward yet.

	Boris Zbarsky [:bzbarsky]
	Comment 19 • 18 years ago

If this is breaking security dialogs on API-stable branches, we need to fix that on those branches one way or another.  Breaking that sort of thing is just NOT acceptable.

	Benjamin Smedberg
	Comment 20 • 18 years ago

bz, what is breaking? The generic code that was checked in did not require any branding. This bug was left open to find a solution so that Firefox didn't have to override the the default, I thought.

	Boris Zbarsky [:bzbarsky]
	Comment 21 • 18 years ago

Is bug 343837 caused by a different problem then?

	Smokey Ardisson (offline for a while; not following bugs - do not email)
	Comment 22 • 18 years ago

netError.dtd did use generic strings, but the changes in bug 236933 used branded strings for the SSL errors.

As we (Camino folks) understood it, branding is branding, whether the thing needing it is netError.dtd or pipnss.properties or whatnot, and that this would be the bug to fix things so that embedding apps would get some sort of branding package by default--so we mentioned this bug in connection with any checkin that added strings utilizing branding to Core strings files.

If that's not the case, I apologize for the confusion and for mis-appropriating this bug; it if is the case, though, perhaps we should probably remove "for netError.xhtml" from the bug summary to prevent further such confusion.

	Benjamin Smedberg
	Comment 23 • 18 years ago

Unless somebody comes up with a way to do a "default branding package", which I think is a very difficult problem (and not something we can deal with on branches), we should instead avoid using branding strings from embeddable code. So the NSS issue would be a bug (but not this bug).

	Smokey Ardisson (offline for a while; not following bugs - do not email)
	Comment 24 • 18 years ago

(In reply to comment #23)
> So the NSS issue would be a bug (but not this bug).

I filed bug 360015 on that.  Sorry, I'd meant to do that after your comment 18 anyway.

	Daniel Veditz [:dveditz]
	Comment 25 • 18 years ago

I don't see how this could block a security release if you guys haven't even agreed on an approach. I'll put on the branch radar but it's not blocking. If you get an acceptable reviewed trunk-landed patch ask for branch approval.

	Daniel Veditz [:dveditz]
	Comment 26 • 18 years ago

Not going to block on this

	Henrik Skupin [:whimboo][⌚️UTC+2] Reporter
	Comment 27 • 17 years ago

Could anyone summarize what we have to do to get this fixed? Or are there no plans at the moment?