Closed Bug 368091 Opened 18 years ago Closed 16 years ago

Toolkit's about:license needs to allow for different "official binaries" line

Categories

(Toolkit :: UI Widgets, defect)

Product:
Toolkit
Component:
UI Widgets
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: alqahira, Assigned: alqahira)

References

(
URL
)

Details

Attachments

(2 files, 4 obsolete files)

even better fix
5.40 KB, patch
benjamin
: review+
gerv
: review+
beltzner
: approval1.9+
Details | Diff | Splinter Review
attempted bustage fix for sunos
1.12 KB, patch
Details | Diff | Splinter Review 
In bug 353917 we replaced the "Official binaries of this product released by the Mozilla Corporation are made available under the corresponding EULA." line (and link) in xpfe's about:license with one more appropriate for non-MoCo products (s/Corp/Found/ and a new link).

When SeaMonkey and Camino start pulling the Toolkit license.html file, the file is going to need to have a way to select the appropriate MoCo or MoFo line (rather than forcing each product to override or fork the file) depending on whether the product in question ships with a MoCo or MoFo EULA.

(It also seems like people pulling/using Mozilla code outside of Mozilla.[org|com] shouldn't be getting a "branded" license that they have to override themselves at all--the default file should be unbranded and our products should have switches to enable the "official binaries" line.  But that isn't really a requirement for this bug.)
> When SeaMonkey and Camino start pulling the Toolkit license.html file

Do we know how soon this is going to be?

bsmedberg: what's the appropriate method of doing such build-specific changes? Is it to have the different lines in separate files, and the build system pick the correct one and insert it into the file? Or some other method?

Gerv

(In reply to comment #1)
> > When SeaMonkey and Camino start pulling the Toolkit license.html file
> 
> Do we know how soon this is going to be?

SeaMonkey is already moving over via "suiterunner", aiui. Camino should follow later, but it's a matter of doing the work. So... "this year, hopefully."

> bsmedberg: what's the appropriate method of doing such build-specific changes?
> Is it to have the different lines in separate files, and the build system pick
> the correct one and insert it into the file? Or some other method?

Since bsmedberg is out at the moment (I believe), Mark, can you respond to this?


And, while plotting, note that there are (not yet shipped) things like browser/LICENSE, and for a few days /LICENSE, that expect toolkit/content/license.html to serve as the license in a source tree, without needing a preprocessor to either make it apply or make it readable.
> bsmedberg: what's the appropriate method of doing such build-specific changes?
> Is it to have the different lines in separate files, and the build system pick
> the correct one and insert it into the file? Or some other method?

No build-time change is going to work (since we're shipping this file in XULRunner, at least in perfect theory). It has to be composited at runtime, or provided by the application.
This would be a great use for sizing-to-content text/html iframes. <sigh>

So we need some sort of external file with this information in which can be substituted at build time (depending on which application you are building) and then included at run time. Can the page XMLHTTPRequest a local file, or would it run into permissions problems?

Alternatively, we could use an iframe and set up a JS communications mechanism to allow the outer page to resize the iframe to the size of its content. As both frames are from trusted sources, this is fine.

Or perhaps there's a really easy solution that I'm not seeing.

Gerv
Any use of JS means you no longer work for Thunderbird and Sunbird, unless you write a license viewer to display it in (in which case you no longer work as a source license, since you require building to see the license).
I don't think that's true. It seems entirely reasonable to me that our license document is an HTML document - all systems these days have some sort of HTML viewer. If you download the source, open the document in your current browser to read it. 

The markup and JS have to be cross-browser, though. Firefox-specific JS would indeed be inappropriate :-)

Gerv
If we're shipping the license.html file as part of "the toolkit", it cannot change between products. i.e. we need to ship one file with XULRunner and have it work for products released under all sorts of licenses. Since this is probably not possible, I don't see any way to avoid shipping a separate file with each app. Though perhaps it may be possible to share the common parts and just have app-specific licensing details.
bsmedberg: having one file with the common parts and including app-specific licensing details from another file (at the top of the page) is exactly what I am suggesting. Or have I missed something about what you are saying?


Gerv
Gerv, I don't know what you're suggesting. You seem to want to be able to open this file in a browser, which doesn't sound compatible with dynamic includes to me.
I'm suggesting either:

<iframe src="product-specific.html">

or (in very pseudo-code):

<div id="content-specific"></div>
...
<script>
  data = new XMLHttpRequest().get("file:content-specific.html")).responseText;
  document.getElementById("content-specific").innerHTML = data;
</script>

I was asking if approach 2 would work. Approach 1 has the problem that the iframe won't size to content.

Gerv

1) a single xulrunner can run multiple client apps with different licenses (although there will probably be a "XULRunner EULA" as well). So including the app file from the shared file won't work.

2) an app isn't necessarily going to know where XULRunner is installed. So including the shared file from the app file isn't going to work (except within the app at runtime)

I'm not sure I understand what about:license is for, anyway: we don't expose it in the Firefox UI, do we?

I think the simple answer to this problem is to have each app ship its own license file (combining shared text at build time, if appropriate).
(In reply to comment #12)
> I'm not sure I understand what about:license is for, anyway: we don't expose it
> in the Firefox UI, do we?

Tail end of the credits in the About dialog.

The problem seems to be that we're deep in implementation here, without actually having design or requirements formalized anywhere (that we know about, anyway).

toolkit/content/license.html is being treated as both a source LICENSE file (see http://lxr.mozilla.org/seamonkey/source/browser/LICENSE) and as an after-the-agreement way to find the EULA, and as a way to discharge our legal requirements in built apps. That may or may not be more than one file can manage.
> Tail end of the credits in the About dialog.

Yes; and it probably needs to be more prominent, legally. (See MPL section 3.6.)

Here's an attempt at a requirement statement:

"The license.html file shall contain all text necessary to discharge our obligations under any license under which the code is shippable, and make users aware of their rights."

So it requires a copy of the MPL, one of the GPL, one of the LGPL, and one of any BSD-like licence which says "A copy of the license needs to be in the supporting documentation". It requires a list of Initial Developers for MPL compliance, and a link to where you can get the source. It also requires a few single-line attributions for other BSD variants.

It doesn't actually have to mention the EULA.

Does that work?

Gerv

But... does it only need to be displayed in the UI, or does it have to be openable separately?
I believe we were thinking about having it open separately (at least, in non-browser apps) for ease of implementation. But either would be entirely acceptable.

Gerv
Attached patch Sample interim fix (obsolete) — — Splinter Review 
Since SeaMonkey is "already" shipping with the toolkit about:license and since Gecko 1.9 is about done, this bug is in need of at least an interim solution that doesn't claim SeaMonkey is released under the MoCo EULA and link only to it.

Here's one option for such an interim solution--or to at least get discussion moving on this again.

Note that if the ultimate aim of this bug is to eliminate all forked copies of this file (e.g., mail/license.html and whatever Sunbird might be doing), we'll presumably also have to accomodate a Mozilla Messaging EULA mention/link, too.
Comment on attachment 307272 [details] [diff] [review]
Sample interim fix

Gerv, what do you think of this?
Attachment #307272 - Flags: review?(gerv)
Smokey: the problem with this is that readers of the page may not know whether their binary came from the Corporation, the Foundation or someone else. Is it possible for us to fix this right first time?

I guess we need a per-product strings file (if we don't have one already), and either include the relevant strings at compile time or at runtime.

Gerv
Attachment #307272 - Flags: review?(gerv) → review-
(In reply to comment #20)
> include the relevant strings at compile time

comment 13 (and now that SM has changed installers, /LICENSE probably should be pointing at it, too, rather than being MPL/NPL)


I don't see any legal issue with the file doing all the tasks listed in comment #13. It does them fine at the moment. Are you saying there's a technical issue?

Gerv

Yes. Not an insurmountable one, just one that probably requires more makefile-fu than is likely to be available before 1.9 stops taking things like this. Requiring rendering HTML isn't unreasonable, but requiring that the HTML renderer have JS enabled is, and the only solution I've come up with so far to have it say different things per product is to not jar it up, but instead have each app makefile cat their official binary line into a var, and then use sed to replace a "<!-- #MOZBINARYEULA# -->" hot-comment with their EULA line-and-link, and then deal with jarring or packaging themselves (and for bonus points, non-browser apps could then s/about:license#/#/ and solve bug 367718).
There maybe some possibility to fix this is a similar manner to bug 366814 but it would need JS and run into bug 349985.
(In reply to comment #13)
> toolkit/content/license.html is being treated as both a source LICENSE file
> (see http://lxr.mozilla.org/seamonkey/source/browser/LICENSE) 

This bit is still going to be slightly problematic with any form of per-app customization of license.html, as the raw-cvs-source version won't be a "complete" file (having a placeholder string for the EULA bit) and, for forms of customization that meet the requirements of non-browser apps, the raw-cvs-source version won't be a ".html" file anymore per se (rather, a ".html.in" file).

Otherwise, Phil (comment 23) and I had similar ideas about how to approach this, but my method keeps the apps from having to do so much work themselves.  More tomorrow....
I have this more-or-less working.

The general idea is that we add a file in each app's config directory (currently called app-license.html) that contains the relevant EULA block for each app.  Then, at build-time, we place that text into license.html, making it suitable for the given app.  To win bonus points from Phil, about:license#foo is replaced by #foo in non-browser apps.  toolkit/content/jar.mn handles packaging for everyone, just as it does now, and everyone in Mozillaland rejoices.  Sort-of.

Issues, in no particular order:

* Sunbird's config folder lives in calendar/sunbird/config but its MOZ_BUILD_APP is calendar.  Either we can special-case it, or we can add a calendar/config folder for this file.  This patch doesn't currently handle Sunbird for that reason, but either fix is easy once we've decided.

* Are there other MOZ_BUILD_APPs in the tree that build toolkit (besides browser, calendar, camino, mail, suite, xulrunner)?  If so, they'll need the app-license treatment yet.

* How do out-of-tree apps work?  Do they set a MOZ_BUILD_APP (their own, or xulrunner), or do we need to make some sort of fallback that prevents a build error and produces a "generic" working license.html?

* This doesn't quite work for eliminating xpfe/global/resources/license.html yet, because xpfe/global/jar.mn gets executed before Camino builds in toolkit/content.  Not something to hold up this patch on (since it does fix everyone else), but if someone familiar with all the build order/deps can suggest a solution, great.  Otherwise I'll probably make Camino pull the toolkit/content/license.html file into our embed-replacements system; the xpfe license.html will still exist, but it can be allowed to bitrot.

* mail/license.html can be deleted at this point (I think), with some changes to http://mxr.mozilla.org/mozilla/source/mail/app/Makefile.in#367 et seq.; I haven't sat down to think about them. (I also haven't checked to see if there are other apps in-tree that have similar setups that might need fixing.)

* Right now the replaced text goes in as one long line (tabs get preserved, but newlines get zapped), which looks ugly in file source, though it's not the end of the world, I don't think.  Someone with stronger sed-/shell-fu, ideas?

* This patch has addition license.html.in/deletion of license.html, but we'd obviously want to do a cvs copy.

* As of yet I've only tested this with browser (and Camino, where the xpfe jarring won't work); since I've been building for about 12 hours this weekend, I'd really appreciate it if some folks with trees for other apps (and other OSes) would check this out.

So, lots of text there, but I think this is a workable solution; comments, suggestions, hints, please--I'm a bit further afield from where I usually work....  Optimistically taking this bug....
Assignee: nobody → alqahira
Attachment #307272 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Comment on attachment 311280 [details] [diff] [review]
proof-of-concept, one license to rule them all

Actually, re-reading all of Benjamin's comments, now I'm not sure if this is an acceptable solution at all :-(
Er, OK... let us know when you've thought it through some more. But, at least, be specific about what this patch doesn't do that you think Ben thinks it should.

Gerv
(In reply to comment #28)
> be specific about what this patch doesn't do that you think Ben thinks it
> should.

(In reply to comment #8)
> If we're shipping the license.html file as part of "the toolkit", it cannot
> change between products. i.e. we need to ship one file with XULRunner and have
> it work for products released under all sorts of licenses. Since this is
> probably not possible, I don't see any way to avoid shipping a separate file
> with each app.

(In reply to comment #12)
> I think the simple answer to this problem is to have each app ship its own
> license file (combining shared text at build time, if appropriate).

Those are the bits I've gotten confused on.  

If I'm reading those comments correctly, making the file both the repository of all licensing/rights information (which IMO should include our vendor-specific EULAs, since they impact user rights--and which is currently the case) [comment 14] and making the file immutable between products [comment 8] seem mutually exclusive. :-(

Must every app's copy of toolkit.jar!content/global/license.html be identical (and must the apps then ship a separate customized copy somewhere, like Tb is currently doing)?  Or must the "toolkit" version (the thing in cvs and/or the license.html generated by --enable-application=xulrunner or equivalent) simply be generic?

(There's also the "how does this work for 'Firefox-on-XULRunner'" question, but since that's not happening for 1.9 and since "non-MoCo apps on toolkit" is happening for 1.9, for the moment I'd like to focus on fixing the latter.)
What I am saying is that we should not ship a toolkit.jar!content/global/license.html, or if we do, it should contain the license information for the Mozilla platform (XULRunner) in its generic state.

Applications, which presumably have additional/different license information (such as EULAs, trademarks, etc), will ship their own, entirely different license.html file, and we will somehow display that file (via a pref?) in about:license instead.
OK, thanks for the clarification, Benjamin.  

The stuff I've written might still be useful for generating each application's own file (not as written, but as a starting point), but since fixing this bug will require code changes to about:license loading and prefs, I'm no longer a suitable assignee (though I'll be glad to help out on the build/license-processing stuff if wanted).
Assignee: alqahira → nobody
Status: ASSIGNED → NEW
toolkit.jar should ideally be identical for every toolkit-based app, so every file in there should be generic for all those apps.
bsmedberg and kairo are right, in that license.html should ship in <appname>.jar, not in toolkit.jar. People using toolkit for their apps should be aware enough of their licensing obligations to make sure they ship one.

Gerv



(In reply to comment #30)
> What I am saying is that we should not ship a
> toolkit.jar!content/global/license.html, or if we do, it should contain the
> license information for the Mozilla platform (XULRunner) in its generic state.

Since it's been a couple of weeks and no-one's stepped up to do the former, here's a patch that does the latter.  (Doing the latter is also useful for the "source license" expectations mentioned in the last part of comment 13.)

This rips out the EULA block for now but adds a placeholder comment string, in case any app wants to do some processing on the file now to add the EULA "back" and ship the resulting file elsewhere.  The placeholder also will ultimately be useful when there's an ability to show a different license file per-app, since the apps can then build their custom license from this shared file.

There's a line at the bottom (about:license#exceptions) mentioning MoFo trademarks very generally; it's not app-specific (and there are at least 3 platform trademarks mentioned on the page that section links to), so I left it in.

As an end result, we should have a license.html that is no longer incorrect for any mozilla.org app.

Benjamin, is this change acceptable to you?
Attachment #311280 - Attachment is obsolete: true
Attachment #313883 - Flags: review?
Comment on attachment 313883 [details] [diff] [review]
interim, non-pref fix: make toolkit/content/license.html generic

Gerv, is this also OK with you?
Attachment #313883 - Flags: review?(gerv)
Attachment #313883 - Flags: review?(benjamin)
Attachment #313883 - Flags: review?
FYI, as part of the one-license-to-rule-them-all conspiracy, on the trunk Tb is now processing toolkit/content/license.html to remove the "about:license" part from anchors/links, and mail/license.html is no more (on the trunk; bug 427316).
Comment on attachment 313883 [details] [diff] [review]
interim, non-pref fix: make toolkit/content/license.html generic

Do we need to add a Firefox override of this file, or does Firefox already override this file?
Sorry if I've missed something, but this is clearly not OK if it breaks builds. I don't see code to replace that placeholder...

Also, just before a release is not the best time to be touching the Firefox-related files here. Write a patch, yes, but it won't get checked in until after 3.0. And then it'll probably need checking in to the hg repo anyway...

Gerv
(In reply to comment #37)
> (From update of attachment 313883 [details] [diff] [review])
> Do we need to add a Firefox override of this file, or does Firefox already
> override this file?

Benjamin, this was the hint that made it all fall into place.  The way we fix this bug (if you agree) is not with a pref, but by using the existing chrome override system.

* toolkit/content/license.html becomes generic, with an HTML comment placeholder to help apps add their own bits if desired (if a consumer of the file doesn't insert a EULA, no one sees anything at all unless they view-source: the page)

* other apps[1] assemble their own copy from an app-license.html file and the toolkit file, and they put it in their appname.jar!/foo/content and use the chrome override to register the assembled file for about:license's chrome url.

I think this satisfies both parts of comment 30, as well as comment 32 and comment 33.  The toolkit version works as a generic license for Mozilla-the-toolkit, Firefox keeps its EULA line, other apps can also add appropriate EULA lines, and the app-specific files ship in appname.jars.

The only drawback I'm aware of is that 

> * Right now the replaced text goes in as one long line (tabs get preserved, but
> newlines get zapped), which looks ugly in file source, though it's not the end
> of the world, I don't think.  Someone with stronger sed-/shell-fu, ideas?

Taking this bug again since I think this solution will stick ;)

[1] Only Fx in this patch; follow-ups for Tb and Sm (and Sb and XR?), and something for Cm just so we can kill xpfe's license, if this method/patch gets approved
Assignee: nobody → alqahira
Attachment #313883 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #314239 - Flags: review?(benjamin)
Attachment #313883 - Flags: review?(gerv)
Attachment #313883 - Flags: review?(benjamin)
(In reply to comment #38)
> Also, just before a release is not the best time to be touching the
> Firefox-related files here. Write a patch, yes, but it won't get checked in
> until after 3.0.

So it's better to ship other products with a "license" file containing incorrect legal/rights/restrictions information? ;)

My goal with the late attachment 313883 [details] [diff] [review], in absence of someone with more knowledge writing the perfect solution, was to come up with a good solution, one that took us from a situation where the file was correct for one consumer and incorrect for every single other consumer of the file (including "the Toolkit"), to a situation where the file was not incorrect for anyone.

Now I have (what I think is) a better solution, and I hope you'll agree that we  should fix this bug so that Firefox 3 does not ship with an incorrect license.html for the Toolkit, and that other Mozilla apps on Gecko 1.9 also don't ship with  license.html files that are incorrect for them (and for the Toolkit) ;) 
Comment on attachment 314239 [details] [diff] [review]
better fix: make toolkit/content/license.html generic, make browser override it

I'm fine with this approach. The build specifics are troublesome:

>Index: browser/base/Makefile.in

>+CHROME_DEPS = $(APP_LICENSE_FILE)

Please make this CHROME_DEPS += $(APP_LICENSE_FILE) and move it after including config.mk

>+PLATFORM_LICENSE_FILE	= $(topsrcdir)/toolkit/content/license.html
>+APP_LICENSE_TEXT_FILE	= $(srcdir)/content/overrides/app-license.html
>+APP_LICENSE_TEXT		= $(shell cat $(APP_LICENSE_TEXT_FILE))
>+APP_LICENSE_FILE		= content/overrides/license.html

I'm really worried/unhappy about catting the contents of that file into a shell makefile. You could easily accidentally add a quote mark or other data to that file which ends up causing the quoting in the sed command below to be confused. I'd prefer that we use the text preprocessor to solve this problem, perhaps like this:

In toolkit/content/license.html:

#ifdef APP_EULA_BLOCK
#includesubst @APP_EULA_BLOCK@
#endif

Then just -DAPP_EULA_BLOCK=$(srcdir/content/overrides/app-license.html and

*  content/browser/license.html        (/toolkit/content/license.html)

>+$(APP_LICENSE_FILE): $(PLATFORM_LICENSE_FILE) $(APP_LICENSE_TEXT_FILE)
>+	mkdir -p $(dir $@)
>+	sed -e "s|<!-- APP_EULA_BLOCK_PLACEHOLDER -->|$(APP_LICENSE_TEXT)|" $< > $@

I think this whole block can be removed, but note that mkdir -p is not portable, you'd have to use $(NSINSTALL) -D
Attachment #314239 - Flags: review?(benjamin) → review-
Attached patch even better fix — — Splinter Review 
This should address all of the previous review comments.
Attachment #314239 - Attachment is obsolete: true
Attachment #314380 - Flags: review?(benjamin)
Comment on attachment 314380 [details] [diff] [review]
even better fix

Gerv, does license.html look OK?  The pre-processor directives disappear in the copy in toolkit.jar, and in Firefox they're replaced with the old EULA block.
Attachment #314380 - Flags: review?(gerv)
Comment on attachment 314380 [details] [diff] [review]
even better fix

>Index: browser/base/jar.mn

>+*+      content/browser/license.html                  (/toolkit/content/license.html)

No need for *+ ... preprocessing implies override behavior, so just use *

>-+  content/global/license.html                (license.html)
>+*+ content/global/license.html                (license.html)

Or here.

r=me with that change
Attachment #314380 - Flags: review?(benjamin) → review+
Comment on attachment 314380 [details] [diff] [review]
even better fix

On the browser/ bits:

[3:49pm] gavin|: sauron: mpa=me
Comment on attachment 314380 [details] [diff] [review]
even better fix

If others are happy, I am. I'm fairly sure this breaks things for Thunderbird, because they now use this file - but hey, they aren't about to release :-)

Gerv
Attachment #314380 - Flags: review?(gerv) → review+
Comment on attachment 314380 [details] [diff] [review]
even better fix

Drivers: this is a relatively low-risk change which makes it possible for non-MoCo apps to customize about:license effectively.
Attachment #314380 - Flags: approval1.9?
(In reply to comment #46)
> (From update of attachment 314380 [details] [diff] [review])
> If others are happy, I am. I'm fairly sure this breaks things for Thunderbird,
> because they now use this file - but hey, they aren't about to release :-)

I'm going to fix that, of course (and not "breaks" in the really bad sense, but in the "this file is a bit messy and not quite right now" sense); I'll file follow-ups as appropriate.
r=me for bending us slightly, since I know I'll like our unbent shape even more.
Blocks: 343544
Comment on attachment 314380 [details] [diff] [review]
even better fix

a1.9=beltzner
Attachment #314380 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in browser/base/Makefile.in;
/cvsroot/mozilla/browser/base/Makefile.in,v  <--  Makefile.in
new revision: 1.22; previous revision: 1.21
done
Checking in browser/base/jar.mn;
/cvsroot/mozilla/browser/base/jar.mn,v  <--  jar.mn
new revision: 1.124; previous revision: 1.123
done
RCS file: /cvsroot/mozilla/browser/base/content/overrides/app-license.html,v
done
Checking in browser/base/content/overrides/app-license.html;
/cvsroot/mozilla/browser/base/content/overrides/app-license.html,v  <--  app-license.html
initial revision: 1.1
done
Checking in toolkit/content/jar.mn;
/cvsroot/mozilla/toolkit/content/jar.mn,v  <--  jar.mn
new revision: 1.42; previous revision: 1.41
done
Checking in toolkit/content/license.html;
/cvsroot/mozilla/toolkit/content/license.html,v  <--  license.html
new revision: 1.21; previous revision: 1.20
done

Thanks everyone for all the work/help on this!  (I'm testing the patch for Tb/bug 428144 now and will file follow-ups on Sm/XR/Cm anon.)
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
That started the SunOS boxen on ports burning, so I checked in this fix to hopefully fix them.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: