Closed
Bug 462254
Opened 16 years ago
Closed 16 years ago
Remove EULA from Firefox installer / .dmg and related bits
Categories
(Firefox :: Installer, defect)
Firefox
Installer
Tracking
()
VERIFIED
FIXED
Firefox 3.1b2
People
(Reporter: Dolske, Assigned: Dolske)
References
Details
(Keywords: verified1.9.0.5, verified1.9.1)
Attachments
(4 files, 2 obsolete files)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #456439 +++ Bug 456439 adds a new "about:rights" page and notification bar. The intention is for that to also replace the current EULA stuff that's shown to Windows and OS X users when installing Firefox. [The about:rights notification bar isn't shown if the user was already shown the EULA.] IIRC, there are also some trivial scattered "EULA" references that we may want to take care of at the same time.
Flags: blocking-firefox3.1?
Assignee | ||
Updated•16 years ago
|
Assignee: nobody → dolske
Updated•16 years ago
|
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.5?
Updated•16 years ago
|
Flags: blocking1.9.0.5?
Flags: blocking1.9.0.5+
Flags: blocking-firefox3.1?
Flags: blocking-firefox3.1+
Assignee | ||
Comment 1•16 years ago
|
||
This removes the changes the OS X .dmg to no longer prompt for a EULA agreement, and removes the licenses from the tree. The code to build a dmg with a license is still there (through we don't use it), maybe Mozilla-derived products will want to use it? Untested, doing builds now. :-)
Attachment #346143 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 2•16 years ago
|
||
Ok, verified that I can build a .dmg with a EULA prompt, and that with this patch that no longer happens. One problem, though. :( The pref removal from firefox.js makes sense for new installs, but will cause existing profiles to be shown the about:rights notification again -- argh. Not sure what the best way is to avoid this. Maybe check something like browser.startup.homepage_override.mstone and set the pref to not show the notification? If there's not a good solution, I wonder how bad it would be to just show the notification to all existing users. :/
Assignee | ||
Updated•16 years ago
|
Attachment #346315 -
Flags: review? → review?(robert.bugzilla)
![]() |
||
Updated•16 years ago
|
Assignee: dolske → nobody
Component: General → Installer
QA Contact: general → installer
![]() |
||
Updated•16 years ago
|
Attachment #346315 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 4•16 years ago
|
||
This fixes the problem noted in comment 2. The specific goal is to set browser.rights.3.shown to true when a OS X or Windows user upgrades across a boundary with the other patches in this bug. [Because we changed the default pref, but don't want to nag users who already saw the EULA in the old installer.] The broader-but-simpler patch is to just always set browser.rights.3.shown when the browser sees that the user is upgrading an existing profile. In other words, having |savedmstone| set means you've got an existing profile, and there's no case where we want to show the about:rights notification in an existing profile. [I could move this outside the |if (mstone != savedmstone)|, but figured this avoids potential Ts impact at the cost of nagging nightly users when we check this in.]
Assignee: nobody → dolske
Attachment #346348 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•16 years ago
|
Target Milestone: Firefox 3.1 → Firefox 3.1b2
Updated•16 years ago
|
Attachment #346143 -
Flags: review?(gavin.sharp) → review+
Comment 5•16 years ago
|
||
Comment on attachment 346348 [details] [diff] [review] [all] Patch v.1 (fix for existing profiles) I'm tempted to say that you should just put this outside the mstone!=savedmstone check... It's very unlikely to make a difference for Ts, and it means we'll get immediate nightly testing of this.
Attachment #346348 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 6•16 years ago
|
||
Pushed changeset e05886ad61db.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 7•16 years ago
|
||
Dolske: fyi (same as in bug 456439), this bug blocks 1.9.0.5, which freezes in one week on November 17. When you have an appropriate 1.9.0 branch patch, please request approval1.9.0.5 on it.
Comment 8•16 years ago
|
||
Oh, it'd also be good to get QA verification of this bug on all relevant platforms before we take it on the 1.9.0 branch.
Comment 9•16 years ago
|
||
Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre. EULA in the installer is gone, and about:rights exists in the notification bar on new profiles.
Status: RESOLVED → VERIFIED
Comment 10•16 years ago
|
||
flagging in-litmus? to add a testcase for the removal of eula in the installer. I believe we already have a testcase for the notification bar.
Flags: in-litmus?
Updated•16 years ago
|
Whiteboard: [needs 1.9.0 patch]
Comment 11•16 years ago
|
||
can we get this automated please? Litmus test added: https://litmus.mozilla.org/show_test.cgi?id=7414.
Flags: in-testsuite?
Flags: in-litmus?
Flags: in-litmus+
Assignee | ||
Comment 12•16 years ago
|
||
I don't think there's a good way of automating this, because most of the change is in the installer/dmg, and the rest of the code only happens on first run in a new profile (this would probably need a new top-level test framework).
Assignee | ||
Comment 13•16 years ago
|
||
Same changes as trunk, currently doing a build running to verify.
Attachment #348666 -
Flags: review?(gavin.sharp)
Updated•16 years ago
|
Attachment #348666 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 14•16 years ago
|
||
Also same as trunk patch, currently building to verify. CVS doesn't seem to realize that browser/EULA.rtf was removed, so the patch doesn't show that.
Attachment #348672 -
Flags: review?
Assignee | ||
Updated•16 years ago
|
Attachment #348672 -
Flags: review? → review?(robert.bugzilla)
![]() |
||
Comment 15•16 years ago
|
||
Comment on attachment 348672 [details] [diff] [review] [Win] Branch patch v.1 Not sure what the approach you are planning to use for the l10n changes but we usually just leave the extra strings for branches.
Attachment #348672 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 16•16 years ago
|
||
Comment on attachment 348666 [details] [diff] [review] [Mac] Branch patch v.1 a? for the [Mac], [Win], and [All] (same as trunk) patches. Shouldn't affect anything outside the installer, same changes as landed on trunk earlier. l10n shouldn't be an issue, not adding any strings here. [The l10n folks are aware of the strings bug 456439 adds to implement about:rights, if that's what you're asking.]
Attachment #348666 -
Flags: approval1.9.0.5?
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs 1.9.0 patch]
![]() |
||
Comment 17•16 years ago
|
||
(In reply to comment #16) >... > l10n shouldn't be an issue, not adding any strings here. [The l10n folks are > aware of the strings bug 456439 adds to implement about:rights, if that's what > you're asking.] I'm not positive (it has been a while since I looked) but I thought the l10n tinderbox reported extra strings.
Assignee | ||
Comment 18•16 years ago
|
||
Oh. Hmm. Axel, is that the case? If so, I'll just land the patch with those strings left in place. [Apparently the freeze is slushy again, so landing tomorrow should be fine.]
Comment 20•16 years ago
|
||
Comment on attachment 348672 [details] [diff] [review] [Win] Branch patch v.1 Approved for 1.9.0.5, a=dveditz for release-drivers -- but please leave the strings in as Axel requested.
Attachment #348672 -
Flags: approval1.9.0.5+
Comment 21•16 years ago
|
||
Comment on attachment 348666 [details] [diff] [review] [Mac] Branch patch v.1 Approved for 1.9.0.5, a=dveditz for release-drivers
Attachment #348666 -
Flags: approval1.9.0.5? → approval1.9.0.5+
Assignee | ||
Comment 22•16 years ago
|
||
Checked in on FF3.0 branch: Removing browser/EULA.rtf; new revision: delete; previous revision: 1.4 Checking in browser/app/Makefile.in; new revision: 1.160; previous revision: 1.159 Removing browser/app/macbuild/license.r; new revision: delete; previous revision: 1.4 Checking in browser/app/profile/firefox.js; new revision: 1.341; previous revision: 1.340 Checking in browser/branding/unofficial/Makefile.in; new revision: 1.7; previous revision: 1.6 Removing browser/branding/unofficial/license.r; new revision: delete; previous revision: 1.4 Checking in browser/components/nsBrowserContentHandler.js; new revision: 1.50; previous revision: 1.49 Checking in browser/installer/Makefile.in; new revision: 1.31; previous revision: 1.30 Checking in browser/installer/windows/Makefile.in; new revision: 1.44; previous revision: 1.43 Checking in browser/installer/windows/nsis/installer.nsi; new revision: 1.46; previous revision: 1.45 Checking in browser/locales/Makefile.in; new revision: 1.67; previous revision: 1.66 Checking in other-licenses/branding/firefox/Makefile.in; new revision: 1.27; previous revision: 1.26 Removing other-licenses/branding/firefox/license.r; new revision: delete; previous revision: 1.4 Checking in toolkit/mozapps/installer/windows/nsis/common.nsh; new revision: 1.44; previous revision: 1.43
Attachment #348666 -
Attachment is obsolete: true
Attachment #348672 -
Attachment is obsolete: true
Assignee | ||
Comment 23•16 years ago
|
||
(Strings mentioned in comments 17-19 were left in the tree. Built an installer with them in place to verify that it didn't cause a problem.)
Assignee | ||
Updated•16 years ago
|
Keywords: fixed1.9.0.5
Assignee | ||
Comment 24•16 years ago
|
||
Note about attachment 346348 [details] [diff] [review]: That patch prevents the problem described in comment 2 and comment 4 (showing the about:rights notification to existing users who already accepted the EULA). The implicit assumption is that an existing profile must have already accepted the old EULA. This makes sense for our builds (which displayed the EULA on install or first-run)... But if the builds in linux distros were suppressing the old EULA, it raises the question if a further fix is needed (in the distro builds) to ensure that their existing users (who have not seen the old EULA) are shown the about:rights notification. Discussed this with beltzner, and decided this was OK as-is. Users with an existing Firefox profile don't need to be shown the about:rights notification, even if they might not have been presented the old EULA. New profiles will, of course, be shown the notification. Also, should we bump the about:rights version in the future, doing so would result in showing the about:rights notification to all users (new and existing, linux distro or not).
Comment 25•16 years ago
|
||
Verified that this is fixed in 1.9.0.5 with Mac, Windows, and Linux builds. None of them get the EULA anymore and they get the "know your rights" infobar with new profiles.
Keywords: fixed1.9.0.5 → verified1.9.0.5
Updated•16 years ago
|
Keywords: fixed1.9.1
Comment 26•16 years ago
|
||
Note: for a Major Update from 2.0.0.20 to 3.0.5 you get a License Text (see Bug 470258). You need to accept this license to do the Major Update. The Problem is that you don't get the "know your rights" infobar after the MU...
Comment 27•16 years ago
|
||
(In reply to comment #26) > The Problem is that you don't get the "know your rights" infobar after the > MU... That was intentional, existing users don't get the infobar since they've already accepted one of the EULAs.
Comment 28•16 years ago
|
||
Though I guess in the major update case they'd have seen only the 2.0 EULA. We can probably fix the check to only skip it for users running a previous 3.0.x version, or just fix bug 470258 such that the about:rights text is shown in the major update offer.
Updated•16 years ago
|
Keywords: verified1.9.1
Updated•16 years ago
|
Keywords: fixed1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•