Closed
Bug 508039
Opened 15 years ago
Closed 15 years ago
Port |Bug 456439 - add about:rights and a "Know Your Rights" infobar to Firefox| to SeaMonkey
Categories
(SeaMonkey :: General, enhancement)
SeaMonkey
General
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.0
People
(Reporter: sgautherie, Assigned: mcsmurf)
References
(Blocks 2 open bugs)
Details
(Keywords: fixed-seamonkey2.0, Whiteboard: [l10n-impact])
Attachments
(2 files, 4 obsolete files)
(deleted),
patch
|
mcsmurf
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
neil
:
review+
kairo
:
approval-seamonkey2.0+
|
Details | Diff | Splinter Review |
(I noticed this while looking at bug 432644.)
SeaMonkey should want to port bug 459439, then bug 462254.
Ftr, TB had
Bug 463367 - Remove EULA from Thunderbird installer / .dmg and related bits
Flags: wanted-seamonkey2?
Reporter | ||
Comment 1•15 years ago
|
||
Comment 2•15 years ago
|
||
Yes, this would be good to have to point users to our license without being intrusive.
Flags: wanted-seamonkey2? → wanted-seamonkey2+
Assignee | ||
Comment 3•15 years ago
|
||
Can this bug actually still be ported for the final version (as it would change/add some new strings)?
Comment 4•15 years ago
|
||
We're not string frozen yet, so why not?
Assignee | ||
Comment 5•15 years ago
|
||
KaiRo: Can you check if the strings are ok as used in this patch? I used the strings from Firefox as a base and removed the ones which talked about phishing and malware detection.
Attachment #400285 -
Flags: ui-review?(kairo)
Attachment #400285 -
Flags: review?(neil)
Comment 6•15 years ago
|
||
Comment on attachment 400285 [details] [diff] [review]
Patch
>+EXTRA_PP_COMPONENTS = \
Except there's no preprocessing in it...
>+ aboutRights.js \
nsAboutRights.js (c.f. nsAboutAbout.js etc.)
>+ifneq (,$(BUILD_OFFICIAL)$(MOZILLA_OFFICIAL))
>+DEFINES += -DOFFICIAL_BUILD=1
>+endif
Except that we don't actually have unofficial branding yet...
>+# ***** BEGIN LICENSE BLOCK *****
XML comment please.
>+#ifdef OFFICIAL_BUILD
IMHO this file belongs in branding; that way when you build unbranded you'll automatically get unbranded rights (when supported). And no #ifdef of course!
>+pref("browser.EULA.version", 3);
Why?
>+pref("browser.rights.version", 3);
>+pref("browser.rights.3.shown", false);
Why 3?
>+#ifdef DEBUG
>+// Don't show the about:rights notification in debug builds.
Why not do this here for unofficial builds too?
>+ var win = this.getMostRecentBrowserWindow();
Ironically the session restore code already knows which window is focused...
>+ // Set pref to indicate we've shown the notficiation.
Nit: notification
>+<!ENTITY rights.intro-point2a "Mozilla does not grant you any rights to the Mozilla and Firefox trademarks or logos. Additional information on Trademarks may be found ">
...
>+<!ENTITY rights.intro-point3a "Privacy policies for &vendorShortName;'s products may be found ">
???
>+<!ENTITY rights.intro-point4a "&brandShortName; also offers optional web site information services, such as the SafeBrowsing service; however, we cannot guarantee they are 100% accurate or error-free. More details, including information on how to disable the services, can be found in the ">
???
Updated•15 years ago
|
Attachment #400285 -
Flags: ui-review?(kairo) → ui-review+
Comment 7•15 years ago
|
||
Comment on attachment 400285 [details] [diff] [review]
Patch
>+<!ENTITY rights.intro-point2a "Mozilla does not grant you any rights to the Mozilla and Firefox trademarks or logos. Additional information on Trademarks may be found ">
>+<!ENTITY rights.intro-point2b "here">
>+<!ENTITY rights.intro-point2c ".">
But we don't have Firefox trademarks in our product :P
You should talk of SeaMonkey trademarks here ;-)
>+<!-- point 3 text for official branded builds -->
>+<!ENTITY rights.intro-point3a "Privacy policies for &vendorShortName;'s products may be found ">
>+<!ENTITY rights.intro-point3b "here">
>+<!ENTITY rights.intro-point3c ".">
We should care to have a bug for those and block 2.0 on them, it's an oversight we don't have them anywhere yet.
>+<!-- point 4 text for official branded builds -->
>+<!ENTITY rights.intro-point4a "&brandShortName; also offers optional web site information services, such as the SafeBrowsing service; however, we cannot guarantee they are 100% accurate or error-free. More details, including information on how to disable the services, can be found in the ">
>+<!ENTITY rights.intro-point4b "service terms">
>+<!ENTITY rights.intro-point4c ".">
We have SafeBrowsing now? OTOH, I wonder why GLS isn't stated there for FF.
The whole branded/unbranded thing doesn't really apply to SeaMonkey right now, by the way, we don't make a difference in our current builds.
Still, no real problems there, so ui-r=me with the comments fixed.
Comment 8•15 years ago
|
||
Pushed changeset 25b3a9e27c15 to comm-central.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 9•15 years ago
|
||
Whoops, commented on wrong bug# :-(
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Updated•15 years ago
|
Status: REOPENED → ASSIGNED
Reporter | ||
Updated•15 years ago
|
Assignee: nobody → bugzilla
Updated•15 years ago
|
Whiteboard: [l10n-impact]
Assignee | ||
Comment 10•15 years ago
|
||
Moved all related files to branding/ (the .xhtml file and in locales/ the .dtd and .properties file). I removed the .EULA.version pref, that was some leftover from some old Firefox JS component. Also I lowered the version number to 1. All other comments have been fixed.
Attachment #400285 -
Attachment is obsolete: true
Attachment #403120 -
Flags: review?(neil)
Attachment #400285 -
Flags: review?(neil)
Comment 11•15 years ago
|
||
Comment on attachment 403120 [details] [diff] [review]
Patch
>+ var notifyRightsText = rightsBundle.formatStringFromName("notifyRightsText",
>+ [productName], 1);
>+
>+ var buttons = [
Nit: blank line isn't quite blank ;-)
Assignee | ||
Comment 12•15 years ago
|
||
Forgot to add two files...
Attachment #403120 -
Attachment is obsolete: true
Attachment #403136 -
Flags: review?(neil)
Attachment #403120 -
Flags: review?(neil)
Comment 13•15 years ago
|
||
Comment on attachment 403120 [details] [diff] [review]
Patch
Well, you forgot to include added files...
>+#ifdef DEBUG
>+// Don't show the about:rights notification in debug builds.
>+pref("browser.rights.override", true);
>+#elifndef OFFICIAL_BUILD
>+// Don't show the about:rights notification in non-official builds.
>+pref("browser.rights.override", true);
>+#endif
No offical release value for this?
>+ case "sessionstore-windows-restored":
>+ this._onBrowserStartup();
>+ break;
So, I don't have a working rights patch to test with, but I think it should be fairly simple to get the session store service to pass the focused window or the last window as the subject of the observation, which would avoid the hacks to recalculate it.
>+ _shouldShowRights : function () {
Nit: too many spaces, should be _shouldShowRights: function() {
>+ var nsIStringBundleService = Components.interfaces.nsIStringBundleService;
Nit: could use const
function(aNotificationBar, aButton) {
>+ browser.selectedTab = browser.addTab("about:rights");
How does the notification get removed?
>+<!ENTITY rights.webservices-term2 "You are welcome to use these Services with the accompanying version of &brandShortName;, and you have all the rights necessary to do so. &vendorShortName; and its licensors reserve all other rights in the Services. These terms are not intended to limit any rights granted under open source licenses applicable to &brandShortName; and to corresponding source code versions of &brandShortName;.">
Nit: no point putting two spaces after a full stop.
Comment 14•15 years ago
|
||
(In reply to comment #13)
> So, I don't have a working rights patch to test with
Now using your latest patch, it's reasonably straightforward to pass the sessionstore's focused window as the subject of the notification.
Comment 15•15 years ago
|
||
Neil, Frank, could we get something to happen here soon? We have a string freeze for the whole series upcoming in about 30 hours...
Assignee | ||
Comment 16•15 years ago
|
||
Ignore the changes in suite/profile/migration/src, somehow managed to modify the wrong patch, will remove this of course.
Attachment #403136 -
Attachment is obsolete: true
Attachment #404129 -
Flags: review?(neil)
Attachment #403136 -
Flags: review?(neil)
Assignee | ||
Comment 17•15 years ago
|
||
http://hg.mozilla.org/comm-central/rev/6e1ba8d29afa
Checked in aboutRights.dtd and aboutRights.properties because of the l10n freeze.
Comment 18•15 years ago
|
||
Comment on attachment 404129 [details] [diff] [review]
Patch
>+ QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIAboutModule]),
>+
Nit: trailing whitespace
>diff --git a/suite/common/public/nsISuiteGlue.idl b/suite/common/public/nsISuiteGlue.idl
Don't need this change any more. r=me with this fixed.
>+ _onBrowserStartup: function(aSubject)
Perhaps call the parameter aWindow?
>+ _showRightsNotification : function (aSubject) {
Nit: still too many spaces here!
>+ // Stick the notification onto the selected tab of the active browser window.
>+ var browser = aSubject.gBrowser; // for closure in notification bar callback
Since you need it in a variable anyway, I wonder whether it's worth passing in the browser as the parameter rather than the window. Either way, please use .getBrowser() instead of gBrowser.
Attachment #404129 -
Flags: review?(neil) → review+
Comment 19•15 years ago
|
||
What's still missing here? Just the checkin? Frank, could you do that?
Assignee | ||
Comment 20•15 years ago
|
||
Attachment #404129 -
Attachment is obsolete: true
Attachment #404930 -
Flags: review+
Assignee | ||
Comment 21•15 years ago
|
||
http://hg.mozilla.org/comm-central/rev/e6822b5c172a
Will file a follow-up to remove the EULA from the installer.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago → 15 years ago
Keywords: fixed-seamonkey2.0
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0
Comment 22•15 years ago
|
||
After update from the 20091004 nightly, I get the infobar, but clicking on it gives me popup "The URL is not valid and cannot be loaded." I'm getting the same when typing about:rights manually. Anything missing from the checkin?
[Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20091007 SeaMonkey/2.0pre]
Comment 23•15 years ago
|
||
The problem is that suite/installer/*/packages files didn't make the name change from aboutRights.js to nsAboutRights.js, so this one is missing.
Assignee | ||
Comment 24•15 years ago
|
||
Doh!
http://hg.mozilla.org/comm-central/rev/176861cdb271
Fixed this
Assignee | ||
Comment 25•15 years ago
|
||
And a followup as I deleted the wrong line http://hg.mozilla.org/comm-central/rev/661df4fe22c7
Reporter | ||
Comment 26•15 years ago
|
||
(In reply to comment #24)
> http://hg.mozilla.org/comm-central/rev/176861cdb271
Nit:
{
2.2 +++ b/suite/installer/windows/packages Wed Oct 07 14:31:06 2009 +0200
2.3 @@ -294,11 +294,11 @@ bin\components\GPSDGeolocationProvider.j
2.10 bin\components\nsAboutFeeds.js
2.11 bin\components\nsAboutSessionRestore.js
2.12 +bin\components\nsAboutRights.js
2.13 bin\components\nsAddonRepository.js
}
Please, move nsAboutRights.js up one line.
Reporter | ||
Comment 27•15 years ago
|
||
Ftr, bug 513023 landed on m-1.9.2+ (only), will we need to do a follow-up about that too?
Assignee | ||
Comment 28•15 years ago
|
||
Hm, that one uses fixed links like
<li>&rights.intro-point3a;<a href="http://www.mozilla.com/legal/privacy/">&rights.intro-point3b;</a>&rights.intro-point3c;</li>
though in toolkit/ for aboutRights.xhtml (not sure if we want that?). Not sure if our about:rights handler would override that about:rights handler?
Reporter | ||
Comment 29•15 years ago
|
||
(In reply to comment #28)
> Not sure if our about:rights handler would override that about:rights handler?
Possibly related: (TB) bug 513025 comment 1 and (TK) bug 514817.
(Maybe just open a follow-up bug ftb?)
Reporter | ||
Comment 30•15 years ago
|
||
(In reply to comment #26)
> Please, move nsAboutRights.js up one line.
I'm eventually fixing this nit in bug 521524.
Comment 31•15 years ago
|
||
Anyone know why about:rights gives XML parse errors in all of my builds?
Reporter | ||
Comment 32•15 years ago
|
||
(In reply to comment #31)
> Anyone know why about:rights gives XML parse errors in all of my builds?
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1.4) Gecko/20091007 SeaMonkey/2.0] (release, rc1) (W2Ksp4)
WorksForMe.
Comment 33•15 years ago
|
||
Comment on attachment 404930 [details] [diff] [review]
Patch for checkin
[Checkin: See comment 21+24+25]
>+ <!ENTITY % securityPrefsDTD SYSTEM "chrome://navigator/locale/pref/pref-security.dtd">
>+ %securityPrefsDTD;
OK, I figured it out: this file doesn't exist; when you're using flat chrome then the document breaks but when you're using jar chrome then it does not.
Comment 34•15 years ago
|
||
(In reply to comment #33)
> OK, I figured it out: this file doesn't exist; when you're using flat chrome
> then the document breaks but when you're using jar chrome then it does not.
Right, it's actually chrome://communicator/locale/pref/pref-security.dtd - where is it actually used? Do we need a patch for 2.0 still? If so, we need to be really fast.
Reporter | ||
Comment 35•15 years ago
|
||
The file exists at
/suite/locales/en-US/chrome/common/pref/pref-security.dtd
chrome://communicator/locale/pref/pref-security.dtd
but is useless here.
Attachment #406022 -
Flags: review?(neil)
Attachment #406022 -
Flags: review?(bugzilla)
Updated•15 years ago
|
Attachment #406022 -
Flags: review?(neil)
Attachment #406022 -
Flags: review?(bugzilla)
Attachment #406022 -
Flags: review+
Reporter | ||
Updated•15 years ago
|
Attachment #406022 -
Flags: review+ → approval-seamonkey2.0?
Updated•15 years ago
|
Attachment #406022 -
Flags: approval-seamonkey2.0? → approval-seamonkey2.0+
Reporter | ||
Comment 36•15 years ago
|
||
Comment on attachment 406022 [details] [diff] [review]
(Cv1) Remove extraneous (and wrong) pref-security.dtd include
[Checkin: Comment 36]
http://hg.mozilla.org/comm-central/rev/45df08842b48
Attachment #406022 -
Attachment description: (Cv1) Remove extraneous (and wrong) pref-security.dtd include → (Cv1) Remove extraneous (and wrong) pref-security.dtd include
[Checkin: Comment 36]
Reporter | ||
Updated•15 years ago
|
Attachment #404930 -
Attachment description: Patch for checkin → Patch for checkin
[Checkin: See comment 21+24+25]
Reporter | ||
Comment 37•15 years ago
|
||
(In reply to comment #21)
> http://hg.mozilla.org/comm-central/rev/e6822b5c172a
>
> Will file a follow-up to remove the EULA from the installer.
I eventually filed bug 525536.
Comment 38•14 years ago
|
||
(In reply to comment #13)
> >+ case "sessionstore-windows-restored":
> >+ this._onBrowserStartup();
> >+ break;
> So, I don't have a working rights patch to test with, but I think it should be
> fairly simple to get the session store service to pass the focused window or
> the last window as the subject of the observation, which would avoid the hacks
> to recalculate it.
Neil, that had the caveat of yourself reviewing the change in bug 558644 that made this stop being done again and broke the notification. We should either port this improvement to Firefox or do it the Firefox way on our side, or we are prone to doing this again...
Comment 39•14 years ago
|
||
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a6pre) Gecko/20100619 Lightning/1.1a1pre SeaMonkey/2.1a2pre - Build ID: 20100619011136
I VERIFY that this build accepts about:rights in the URL bar, responding with a short page titled "About Your Rights" with a list of 4 items for license, trademarks, privacy policy and service terms. I checked that the first three links go to pages with what looks like adequate content (though not yet finalized in the case of the privacy policy); the fourth one opens an additional section on the same about:rights page: "SeaMonkey Website Services" with a heading paragraph and a six-point numbered list.
I am not sure in what circumstances the infobar mentioned in the Summary should appear.
Comment 40•14 years ago
|
||
(In reply to comment #39)
> I VERIFY that this build accepts about:rights in the URL bar
Yup, that hasn't been broken and should still work fine.
> I am not sure in what circumstances the infobar mentioned in the Summary should
> appear.
It should appear on first launch (possibly any first launch of a new version, I don't remember the heuristics in detail - surely first launch of a new profile, though) and that is what is broken due to bug 558644 changes and for which I filed bug 573384 now.
You need to log in
before you can comment on or make changes to this bug.
Description
•