Closed Bug 302309 Opened 15 years ago Closed 4 years ago

Embedded applications still need their own branding for netError.xhtml

Categories

(Core Graveyard :: Embedding: Packaging, defect)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: whimboo, Unassigned)

References

Details

Attachments

(1 file)

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.
No longer depends on: 280190
Depends on: 280190
my guess is conrad shouldn't be the owner.
Assignee: ccarlen → nobody
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Attachment #190996 - Flags: review?(cbiesinger)
Blocks: 301208
Attachment #190996 - Attachment description: Using brandFullName in netError.xhtml → Core part for using brandFullName in netError.xhtml
Assignee: hskupin → nobody
Status: ASSIGNED → NEW
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
Attachment #190996 - Flags: review?(cbiesinger) → review+
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?
Attachment #190996 - Flags: review?(benjamin)
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.
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)
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.
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?)
Attachment #190996 - Flags: review?(benjamin) → review-
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.
So... it looks like this missed 1.8.  What's out 1.8 story for embeddors at this point?
Flags: blocking1.8.1?
(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
I thought we fixed this by making the docshell netError.xhtml generic and overriding it from Firefox...
If we did, great.  Please resolve this as appropriate.
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.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → INVALID
reopening, that's just a workaround and we should fix this.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Not going to block 1.8.1 for this bug.
Flags: blocking1.8.1? → blocking1.8.1-
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)
Flags: blocking1.9?
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.
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.
Severity: normal → major
Flags: blocking1.8.1.1?
Flags: blocking1.8.0.9?
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.
Is bug 343837 caused by a different problem then?
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.
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).
(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.
No longer blocks: 343837
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.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1-
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9-
Not going to block on this
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x?
Flags: wanted1.8.0.x+
Flags: wanted1.8.1.x?
Flags: wanted1.8.0.x?
Could anyone summarize what we have to do to get this fixed? Or are there no plans at the moment? 
Flags: blocking1.9? → blocking1.9-
Assignee: nobody → nobody
QA Contact: packaging
Status: REOPENED → RESOLVED
Closed: 15 years ago4 years ago
Resolution: --- → INCOMPLETE
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.