Closed Bug 423104 Opened 12 years ago Closed 12 years ago

Ship dictionary READMEs

Categories

(Core :: Spelling checker, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: u60234, Assigned: Gavin)

Details

Attachments

(3 files, 5 obsolete files)

The Readme files for the spell checking dictionaries we have in our tree is not shipped together with the .dic and .aff files in our products.

As I understand it, some of the dictionaries have licences that require this. See for example http://mxr.mozilla.org/mozilla/source/extensions/spellcheck/locales/en-US/hunspell/README.txt#34
Flags: blocking1.9?
Summary: Ship dictionary README's → Ship dictionary READMEs
Kevin, what's your take on this?
Yes, we should ship the readme files that are required by dictionary licenses. 
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
mscott, is this on your radar?
(In reply to comment #3)
> mscott, is this on your radar?

Doubt it. He's just the default assignee for this component.
Assignee: mscott → nobody
Assigning to MConnor - can you find a better owner for this?
Assignee: nobody → mconnor
We need to include the text below in our Read-Me file or legal notices file, or the location where we commonly include such copyright attributions. I checked the read-me file and the only requirement is attribution and inclusion of the permission statements, pretty common, except in this case its a collection of permissions with a bunch of correspondence intermixed within, making it confusing. 

Thus, in short, as Damon said above, ship the entire read-me file for the dictionary at:

http://mxr.mozilla.org/mozilla/source/extensions/spellcheck/locales/en-US/hunspell/README.txt#34
The problem with "the location where we commonly include such copyright attributions" (which is about:licence) is that it's the same across all shipped versions of Firefox, whereas this file only applies to the en-US version. So just pasting it all in there, as well as being ugly and looking unprofessional (in its current form) would be inaccurate in the majority of shipped Firefoxes.

We also don't have a definitive list of which versions of Firefox ship with dictionaries. Axel or Mic: can you help with that?

We should take that en-US README and prune unnecessary parts, shipping only the legally-required license declarations.

We could do some sort of dynamic include mechanism which picked up any licensing documents installed in a particular place, or referenced in a particular pref tree. However, about: protocol URLs can't access chrome (right?), which makes that much harder.

Another option would be pasting the lot in, and hiding the irrelevant ones using JavaScript by checking the User-Agent. That would be hacky, but it would work.

Gerv
With a little magic you can probably use DTDs to include l10n-specific license blocks in the file:

<!ENTITY % localeLicenseDTD SYSTEM "chrome://browser/locale/license.dtd">
%localeLicenseDTD;

And down near the bottom of about:license

&locale.license;
Sorry, I'm not finding sufficient time to devote to this issue :-|

bsmedberg: sorry to be thick; are you saying that the markup you've just quoted works fine in a standard HTML file?

(If so, why don't more people use it on the web to do inclusions?)

How might we also add a ToC entry for the licence? Perhaps some JS in the included file which adds it? Or would such JS not run because of the include mechanism used? Could the included file actually have any markup in it at all?

Gerv
XHTML, not HTML. Doesn't work on the web because we don't load remote DTDs, only local (chrome) ones.

The ToC entry could just be another entity from the same DTD. And the case of no dictionary license the included file wouldn't have "no markup", it would have empty DTD definitions:

<!ENTITY locale.license "">
Assignee: mconnor → benjamin
Which licenses require a license statement? Glancing over http://wiki.mozilla.org/L10n:Dictionaries

fr, hu, id, pl pt-PT, ru, sk, sr, sv-SE, uk: triple license

lt, nl: BSD
nl: cc-by
ru: cc-sa

mk: Public domain

ta: en-US dictionary

ro: seems from http://rospell.sf.net, and GPL only?
Axel: which of those do we ship? All of them? If so, we have a problem with ro.

Gerv
bsmedberg or anyone: again, I must be thick, but I can't get this to work. 
I'm testing it on an install of 3.0b5. I've unpacked my chrome and hacked the manifests so chrome://browser/locale/license.dtd works. I've then tried putting the magic commands in various places in the license.html file. I've tried switching the DOCTYPE to an XHTML one, as bsmedberg said that it only works with XHTML. I tried putting the entity references in the main body, and inside the DOCTYPE. I can't get it to work any way round - even for local references. 

%localeLicDTD; always prints as text rather than being substituted. Firefox doesn't like:
<!DOCTYPE html
PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd" [
<!ENTITY % localeLicDTD SYSTEM "chrome://browser/locale/license.dtd">
%localeLicDTD;
]>
either - the final ]> prints as text, implying that it doesn't understand this form.

What am I doing wrong?

Gerv
OK, the file was being interpreted as HTML. I have to rename it (!) to get it to show up as XHTML. And this, of course, as a load of ramifications. I'm working on a patch which I hope will do the trick.

Gerv
Attached patch Patch v.1 (obsolete) — Splinter Review
Here's a work in progress. It moves the .html to a .xhtml (which is required for Firefox to treat it as XHTML :-( ) and updates what I hope are the correct references. I've CCed Smokey so he can review that part. Then, it changes the license.x?html file so that it actually is XHTML, which involves adding a lot of closing tags and fixing a few entity references. Lastly, it adds bsmedberg's magic sauce to the top of the file.

Open question: where in the tree do the files live which will end up as chrome://browser/locale/license.dtd ? And what build changes need to be made to make sure it ends up in the right place?

bsmedberg: I didn't phrase one of my previous questions well. Can the included text from the DTD, i.e. the ??? in <!ENTITY locale.license "???">, contain HTML markup? Or can it only be plain text? Just plain text might present a problem...

Gerv
Filed bug 430946 on the romanian dictionary.

Regarding the technicalities: Yes, you can put xml markup into the entity values. http://mxr.mozilla.org/mozilla/source/browser/locales/en-US/chrome/overrides/netError.dtd is an example of what's possible. The simpler the better still, of course.

mozilla/browser/locales/en-US/chrome/browser/ sounds like a good place to put it, and it needs to be added to mozilla/browser/locales/jar.mn to be built.

The DTD file itself should contain a localization note with concrete instructions on what to do, as usually the worst possible thing would be to translate the English text. http://developer.mozilla.org/en/docs/Localization_notes has docs on that.
Entities can contain valid complete markup: that is: an entity can be

<!ENTITY foo "text">
<!ENTITY foo '<a href="foo">text</a>'>

But not
<!ENTITY foo '<a href="foo">text'> <!-- invalid, no closing </a> -->
Comment on attachment 317881 [details] [diff] [review]
Patch v.1

I've just taken a quick look at this; I'll look more thoroughly later in the weekend.

>Index: toolkit/content/license.xhtml
>===================================================================
>RCS file: toolkit/content/license.xhtml
>diff -N toolkit/content/license.xhtml
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ toolkit/content/license.xhtml	26 Apr 2008 13:37:25 -0000
>@@ -0,0 +1,2458 @@
>+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
>+                     "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"
>+ [
>+    <!ENTITY % localeLicenseDTD SYSTEM "chrome://browser/locale/license.dtd">
>+    %localeLicenseDTD;
>+ ]

1a) This won't work with non-Firefox applications as the patch is now (or non-browser apps will have to override it, but it seems to me even in that case the reference in the license file should be to a chrome://global URI); IIRC the current state of the patch will cause the "yellow screen of death" in non-Firefox apps.

1b) How does this work in non-browser applications?  Since a (lightly-processed) license.xhtml will be opened in an arbitrary web browser, the undefined entities should cause the page to error everywhere. 

1c) How do you get the appropriate dictionary license into this file for non-en-US non-browser applications that ship dictionaries, given that referencing a entity from a dtd in a jar isn't going to work?

>Index: xpfe/global/jar.mn
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/global/jar.mn,v
>retrieving revision 1.111
>diff -5 -p -u -r1.111 jar.mn
>--- xpfe/global/jar.mn	15 Jan 2008 21:51:58 -0000	1.111
>+++ xpfe/global/jar.mn	26 Apr 2008 13:37:26 -0000
>@@ -33,11 +33,11 @@ toolkit.jar:
>         content/global/printProgress.xul            (resources/content/printProgress.xul)
>         content/global/printProgress.js             (resources/content/printProgress.js)
>         content/global/printPreviewProgress.xul     (resources/content/printPreviewProgress.xul)
>         content/global/printPreviewProgress.js      (resources/content/printPreviewProgress.js)
> *       content/global/about.xhtml                  (resources/content/about.xhtml) 
>-        content/global/license.html                 (resources/content/license.html)
>+        content/global/license.xhtml                (resources/content/license.xhtml)

2) This change is going to break the build xpfe apps unless there's a corresponding file addition/deletion that's not in this patch.

>Index: suite/common/jar.mn
>===================================================================
>RCS file: /cvsroot/mozilla/suite/common/jar.mn,v

3) There's a comment in suite/common/Makefile.in that mentions the chrome URIs; please fix the extension in those references: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/suite/common/Makefile.in&rev=1.6&mark=63-64#63

4) If the license file ends up being xhtml, should all the app-license.html files become app-license.xhtml to remind everyone that they need complete, correct markup in those files (e.g., I seem to have checked in Fx's and Tb's without the corresponding </p> :-( and have just filed fixes) and make mistakes in those files plainly evident?
Also, item 1b prevents toolkit/content/license.(x)html from functioning as a "source license" file in tarballs, etc. (see bug 368091 comment 3), since those entities won't be defined in a source file.
Also, can we do a CVS copy to keep the history of license.html? I'd rather not lose it, as that's pretty important.
Smokey: gotta go to bed, but: if the patch sucks, are you saying the entire approach is wrong, or that we just need to fix some stuff? If the approach is wrong, what would work better?

Gerv
The only thing wrong with the approach, as opposed to the execution, is that it will make me re-fork Thunderbird's very recently unforked license.html. We open ours in the user's default browser, so this XHTML with a chrome DTD would YSoD in Firefox 2, would show a dictionary license which would only be correct if the user happens to have the same locales which shipped the same dictionaries in both Tbird and Fx 3, and would be completely unopenable if the user's default browser is Internet Explorer.

However, if we stand by the "we'll ship dictionaries which require attribution in about:license, and we must show only the attribution for the particular dictionary which shipped with the locale in use at the time the user views about:license, even though we show the Sparkle license for Firefox on Windows" decision, long run Thunderbird would probably wind up unforking, after we write a license-browser app for toolkit to optionally ship for non-browsers (and to always ship with XULRunner, since at ship-time it doesn't know whether it will wind up running a non-browser). Or, maybe we would just decide not to ship any dictionaries with any locale, not sure which.

Oh, and don't forget that this approach makes the One True License File unacceptable as a source license, so you'll need to fork yourself to also have a /license.txt|.html, because it's not reasonable to require someone who uses IE to download a tarball to first build Firefox in order to find out the license for the code he downloaded.
(In reply to comment #18)
> IIRC the
> current state of the patch will cause the "yellow screen of death" in
> non-Firefox apps.

Actually, since the entities aren't currently being used in the file, it only yellow-screens right now because of bug 430975/bug 430974, the unclosed <meta> tag, un-escaped <> around Jason Evans's email address, and two <hr> that didn't get closed.  However, as soon as those are fixed and we start including &locale.license; in the file somewhere, it does fail.

Also, because of 1a, when I feed the Tb license.xhtml file with the above remaining errors fixed (but no &locale.license; entity included) to Camino 2.0a1pre or SeaMonkey 2.0a1pre, both yellow-screen on the &ouml; entity in Bj&ouml;rn Jacke.  Phil tells me there's a bug out there on things breaking differently depending on whether just the dtd file is missing or whether the whole (or part) of the path to the dtd is missing.

> 1c) How do you get the appropriate dictionary license into this file for
> non-en-US non-browser applications that ship dictionaries, given that
> referencing a entity from a dtd in a jar isn't going to work?

I don't know well enough how generating l10n builds work, but if there isn't a full rebuild of the app with the appropriate language -D dedine set, aren't the localized builds of, e.g., Tb, just going to be repacking the same version of license.xhtml that was created in the main (en-US) build? 

(In reply to comment #21)
> Smokey: gotta go to bed, but: if the patch sucks, are you saying the entire
> approach is wrong, or that we just need to fix some stuff? If the approach is
> wrong, what would work better?

Initially I thought just some stuff needed fixing; 1a and 2-4 were simple fixes.  However, in rebuilding apps this afternoon with the patch, it seems like there are two many probable show-stoppers.

If there were some way of doing entity cascading where we could define dummy entities in the license.xhtml file itself but have a later-in-the-file-referenced chrome dtd URI be used by apps where the URI was valid, we could maybe solve 1b.  Unfortunately, it seems that if I define a dummy entity and then reference the DTD, Firefox picks up the dummy entity and not the entity from its DTD (I added the DTD and the appropriate override in browser/locales/jar.mn when testing).

Switching to preprocessing license.(x)html could solve 1b, but that still leaves 1c and a new variant of 1c, the fact that now Firefox is stuck either including en-US dictionary license in all locales, or including all dictionary licenses in all locales.

Given that about:license currently contains, as Phil mentions, license declarations/attributions for all types of things that don't ship in any particular product (e.g., the Sparkle license shows up in Firefox-Win while the Sparkle-licensed code is only used in Camino, and the xdg license shows up in Camino while that code is only used on non-Mac UNIX-like platfoms), it seems to me the best way to fix this is to just add every dictionary that requires attribution to license.html and be done with it.  This is particularly true if one or more of these dictionaries ship with "the Toolkit"/XULrunner; do they?

(You could conceivably also preprocess this in per-product, so Fx, Tb, Sm could get the chunk of dictionary licenses while Camino, which doesn't ship any dictionaries, would avoid adding said chunk, but that seems like a lot of work for little gain.  The only reason we did preprocessing for the EULA block is that mentioning the wrong EULA for an app was wrong; it "imposed" the wrong legal requirements on users of other apps.)

**Executive summary**

The current approach is unwieldy and unsuitable given the requirements we have placed on about:license/license.html.  The simple, it-sucks-the-least approach (in my opinion) is to ship all dictionary licenses which require attribution in license.html by default, full-stop.
netError.dtd does use entity overloading in DTDs, so I would think that's possible.

I did wake up with similar concerns like the previous comments, in particular with language packs in my head. Those, as of now, don't contain dictionaries, but they would contain the license information to it, if we'd do what I suggested in comment 16.

I see two ways out:

- ship all license disclaimers we need

- put the license extension stuff into a resource:// file, like we do with browserconfig.properties.

The latter would assume that our parsing doesn't go bad on a 404 for that resource file, so that we don't end up in hell on all products.
(In reply to comment #24)
> netError.dtd does use entity overloading in DTDs, so I would think that's
> possible.

It does? IIRC, overloading breaks XML and what we use there instead is a chrome file override.
(In reply to comment #24)
> netError.dtd does use entity overloading in DTDs, so I would think that's
> possible.

It looks like netError.xhtml imports multiple DTDs and then has one of those DTDs import another yet another one for override purposes, whereas here we'd have to do an in-xhtml entity definition (of dummy blank entities) and then import a DTD, similar to how developers use CSS to make a page passable in an old browser and nice in a newer on that supports @import:

/* rules for browsers that don't contain chrome://global/locale/license.dtd */

p {foo}
div {foo}

/* real styles for browsers that contain chrome://global/locale/license.dtd */

@import url("realstyles.css"); 

The only problem is that doesn't seem to work here for entities like it does on the web for style rules.  I'm not very familiar with use of entities and DTDs, so maybe my experiment last night was structured incorrectly and prevented overloading/fallback from working.

Regardless, getting that working won't help non-browser apps ship the "right" dictionary licenses in their license.(x)html; it'll only mean non-browser apps won't be forced to re-fork license.(x)html just from our making browser apps ship the "right" dictionary licenses per locale.
I have been playing around with this a bit on 
http://ted.mielczarek.org/code/mozilla/xuledit/xuledit.xul, which sadly enough, works fine to test this. I played with XUL, but xhtml will do the same thing:

Resolutions:

You can have multiply defined entities, it picks the first one it finds. At least that's what happened in the scenarios I tested with.

What doesn't work is referencing a resource:// DTD, that yields a parsing error.

<?xml version="1.0"?>
<!DOCTYPE window [
<!ENTITY % appLicenseDTD SYSTEM "resource://app/app-license.dtd">
%appLicenseDTD;

<!ENTITY license "">

]>
<?xml-stylesheet href="chrome://global/skin/" type="text/css"?>
<window id="yourwindow" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<label value="&license;"/>
</window>


I guess that going just for a single full blown license file is easiest at this point.
Whiteboard: [ETA 4/29]
Chaps,

Lots of personal things have come up and, rather than pretend I can carry on iterating this patch, it seems that the best thing to do is get out the way ASAP and let someone else work on it. 

I would hate an enormous licence file with the licences for six different dictionaries in. It would be confusing and bloated, particularly as e.g. the en-US dictionary has such complex licensing. (Don't just copy and paste that README either - trim it to retain only the entirely relevant info, not the chatty commentary.) 

If you do something like that, at the very, very least, add some JS which checks the User Agent's language and hides the licences for the inappropriate dictionaries. In a tiny number of cases, that'll be wrong, but it doesn't matter.

I'll try and be available to review any patch to license.html.

Gerv
The useragent locale has little to do with the code that we shipped. I am preparing a patch which unconditionally includes the two licenses which need to be included (nl and lt). Axel, can you file a separate bug on removing the Russian dictionary, if it really is GPL-only?
Attachment #317881 - Attachment is obsolete: true
Attachment #318450 - Flags: superreview?
Attachment #318450 - Flags: review?(alqahira)
Attachment #317881 - Flags: review?(alqahira)
Attachment #318450 - Flags: superreview? → superreview?(handerson)
Whiteboard: [ETA 4/29] → [has patch][needs review handerson+smokey]
(In reply to comment #16)
> Filed bug 430946 on the romanian dictionary.

... assuming that you talk about Romanian and not Russian, referring to comment 11 and 29.
Attachment #318450 - Flags: review?(gerv)
Comment on attachment 318450 [details] [diff] [review]
Add nl, lt license blocks, rev. 1

I guess two extra is not so bad. Two comments:

- The licences are in the file in the same order as the contents page, so please shift the Dutch one upwards to the right place.

- Please change the intro text to model the form used for Sparkle. So e.g.:

This licence applies to certain files in the directory l10n/nl/extensions/spellcheck/hunspell/. (This code only ships in the in the Dutch version of this product.)

Are we sure these are the only two? en-US looked like it had really complicated requirements, including some BSD-like licences...

Gerv
Attachment #318450 - Flags: review?(gerv) → review+
Comment on attachment 318450 [details] [diff] [review]
Add nl, lt license blocks, rev. 1

This looks good to me, too; r=ardissone.

Note that I haven't yet made the xpfe/global/resources/content/license.html version vanish yet (bug 428333), so in theory it needs to be kept in sync with the toolkit copy until that point (the patch applies OK there).
Attachment #318450 - Flags: review?(alqahira) → review+
Comment on attachment 318450 [details] [diff] [review]
Add nl, lt license blocks, rev. 1

>Index: toolkit/content/license.html
>       <li><a href="about:license#ucal">University of California License</a></li>      
>+
>     </ul>

Actually, we probably don't need that blank line there. ;)
Attachment #318450 - Flags: superreview?(handerson) → superreview+
In all this slew of r+ and sr+, someone *is* going to notice that Gerv was right, and that the en-US dictionary requires *four* BSD-ish license blocks, aren't they?
Comment on attachment 318450 [details] [diff] [review]
Add nl, lt license blocks, rev. 1

r-'ing just to make sure comment #35 is dealt with. ;)
Attachment #318450 - Flags: review-
Comment on attachment 318450 [details] [diff] [review]
Add nl, lt license blocks, rev. 1

Never mind. The attachment description says only nl/lt.
Attachment #318450 - Flags: review-
Whiteboard: [has patch][needs review handerson+smokey] → [has patch for nl/lt][needs patch for en-US]
So, do we need to add anything per comment 35 ?  Or, can we get this landed?
I hope I'm not just adding noise, but looking back at comment 11 and Gerv's earlier comment, there were issues with:

lt, nl, ru, ro and en-US

Bug 430946 is dealing with ro (by removing it) (bug is nominated to block) 

The patch here is ready to land, but it deals only with lt and nl.

ru (Russian) - bsmedberg asked if a bug was filed, and Axel answered if he meant Romanian, and then there was no follow up.  Apparently the license is CC-SA, in which case I guess a patch is needed to do the attribution.

en-US: Needs stuff done also, but isn't covered by the current patch.
Comment 11 has duplicate entries, for whichever reason, the russian dictionary is triple license *and* cc-sa.
Pardon my causing confusion: the Russian dictionary is multi-licensed under *4* licenses, see http://lxr.mozilla.org/l10n/source/ru/extensions/spellcheck/hunspell/license.txt

I therefore don't think it needs any special license block in about:license.

I am planning on landing the lt/nl patch today, after fixing the nits.

I didn't realize English needed anything; I was going by the list in comment 11. I am having trouble wading through the multiple license files and many multiple license blocks in http://mxr.mozilla.org/mozilla/source/extensions/spellcheck/locales/en-US/hunspell/README.txt... I'm hoping that Harvey or Gerv can provide a text to add to about:license
"Below are the license and copyright notices associated with the SCOWL dictionaries for the English language, including additional commentary provided by the authors (interspersed) regarding the license permissions."


Is the above the kind of text you were thinking of?
yeah, presumably with the commentary removed and consistent with the rest of about:license... I'd guess we don't have to put in any of the public-domain stuff (since that's not a license) but only the BSD-style licenses.
Comment on attachment 318450 [details] [diff] [review]
Add nl, lt license blocks, rev. 1

Let's get this in..
Attachment #318450 - Flags: approval1.9+
Keywords: checkin-needed
"I am planning on landing the lt/nl patch today, after fixing the nits." <-- I'll let benjamin land this himself. If he wants to try to address the nits, I can do that; just re-add the checkin-needed keyword.
Keywords: checkin-needed
I've landed the lt/nl changes. Reassigning to Harvey for a license text to use for en-US... feel free to assign back to me once that's ready.
Assignee: benjamin → handerson
Whiteboard: [has patch for nl/lt][needs patch for en-US] → [needs patch for en-US][patch for nl/lt landed]
The en-US license language is at http://mxr.mozilla.org/mozilla/source/extensions/spellcheck/locales/en-US/hunspell/README.txt#34. See Comment 6 for license text and review. 

Insert the following above the license:
"Below are the license and copyright notices associated with the SCOWL
dictionaries for the English language, including additional commentary provided
by the authors (interspersed) regarding the license permissions."

Is this what your looking for?


No, I don't think so. That file has snippets of email conversations, public domain annotations, and other stuff that I don't think we want in about:license. Are we required to put the commentary, email snippets, and other bits in about:license?
Attached file Trimmed en-US licence text (obsolete) —
Something like this, I think.

Gerv
-> gavin since I'm going on paternity leave
Assignee: handerson → gavin.sharp
Well done Gerv. Attached is revised document with a few minor tweaks. My concern previously was taking out all the "other non-license" stuff that the author wanted to include. Adding the link to the full file solves this issue.
Attachment #319452 - Attachment is obsolete: true
Attached patch en-US patch (obsolete) — Splinter Review
This adds the text from attachment 319466 [details] into license.html. I picked the name "US English Spellchecking Dictionary", and referred to the source location as "extensions/spellcheck/locales/en-US/hunspell/".

Not sure who needs to review this...
Attachment #319474 - Flags: superreview?(mconnor)
Attachment #319474 - Flags: review?(gerv)
Status: NEW → ASSIGNED
Priority: P2 → P1
Whiteboard: [needs patch for en-US][patch for nl/lt landed] → [has patch][needs review][patch for nl/lt landed]
Target Milestone: --- → mozilla1.9
Comment on attachment 319474 [details] [diff] [review]
en-US patch

>+Additional Contributors:
>+ Alan Beale <biljir@pobox.com>
>+ M Cooper <thegrendel@theriver.com>

Those <> need to be &lt; and &gt; for both.

Be sure to add this patch to xpfe/global/resources/content/license.html to keep those in sync, too, since I imagine this will go in far before bug 428333 does.

I didn't do a full review of this, but if Gerv's unavailable, blame and preceding patches on this bug seem to indicate I can fill in.
Comment on attachment 319474 [details] [diff] [review]
en-US patch

r/sr=me, with smokey's comments (thanks Smokey!)
Attachment #319474 - Flags: superreview?(mconnor) → superreview+
Attached patch updated en-US patch (obsolete) — Splinter Review
Thanks, Smokey. I diffed attachment 319466 [details] and the text copied from about:license in my build to make sure there weren't any more of these problems.

I'll leave the r?gerv but request approval with r=mconnor, I can address any comments Gerv might have after the fact.
Attachment #319474 - Attachment is obsolete: true
Attachment #319578 - Flags: review?(gerv)
Attachment #319578 - Flags: approval1.9?
Attachment #319474 - Flags: review?(gerv)
Whiteboard: [has patch][needs review][patch for nl/lt landed] → [has patch][needs approval][patch for nl/lt landed]
Attached patch en-US patch to xpfe's version (obsolete) — Splinter Review
I noticed there are other discrepancies between these two files, are there existing bugs on that?
Whiteboard: [has patch][needs approval][patch for nl/lt landed] → [has patch][needs review gerv]
Whiteboard: [has patch][needs review gerv] → [has patch][needs approval]
(In reply to comment #56)
> I noticed there are other discrepancies between these two files, are there
> existing bugs on that?

Not sure, the xpfe version is used by Camino only on trunk nowadays - and actually, we all hope they abandon xpfe completely before shipping anything 1.9-based...
Comment on attachment 319578 [details] [diff] [review]
updated en-US patch

>+      <li><a href="about:license#hunspell-en-US">US English Spellchecking Dictionary</a></li>

Can we have this list in alphabetical order, please? While you are there, please fix the Lithuanian contents entry which was put in the wrong place by the previous patch, despite me asking. (I don't know if the licence itself is in the wrong place).

>+Different parts of the US English dictionary (SCOWL) are subject to the following licenses as 
>+shown below.  For additional details, sources, credits, and public domain references, see 
>+http://mxr.mozilla.org/mozilla/source/extensions/spellcheck/locales/en-
>+US/hunspell/README.txt?raw=1

As this file is HTML, please can we have this as a link, please? Something like:

For complete details, see <a href="...">README.txt</a>.

>+The collective work of the Spell Checking Oriented Word Lists (SCOWL) is under the 
>+following copyright:
>+Copyright 2000-2007 by Kevin Atkinson

Can we please have the blank lines back? It looks _really_ ugly without them.

Gerv
Attachment #319578 - Flags: review?(gerv) → review+
I fixed the ordering of the ToC items, but not the items themselves, since there are others that are out of order. I added the link, but didn't touch the spacing. We can address those in non-blocking followup bugs if they're really needed.
Attachment #319578 - Attachment is obsolete: true
Attachment #319595 - Attachment is obsolete: true
Attachment #319629 - Flags: approval1.9?
Attachment #319578 - Flags: approval1.9?
Comment on attachment 319629 [details] [diff] [review]
updated patch, including xpfe

a1.9=beltzner
Attachment #319629 - Flags: approval1.9? → approval1.9+
Whiteboard: [has patch][needs approval] → [has patch][has approval]
mozilla/toolkit/content/license.html 	1.23
mozilla/xpfe/global/resources/content/license.html 	1.20

FIXED.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has approval]
You need to log in before you can comment on or make changes to this bug.