Closed Bug 1328445 Opened 3 years ago Closed 3 years ago

Streamlined stub installer

Categories

(Firefox :: Installer, defect)

All
Windows
defect
Not set

Tracking

()

VERIFIED FIXED
Tracking Status
firefox51 + wontfix
firefox52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- verified

People

(Reporter: agashlin, Assigned: agashlin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fce-active-legacy])

User Story

As a Windows user, I want Firefox to install as quickly as possible so I can start browsing.

Acceptance criteria:
- Stub installer file name changed to "Firefox Installer"
- Stub installer caption logo changed to Firefox logo
- Stub installer file icon changed to Firefox logo
- Modal dialogue displayed on download cancel or install error replaced by “.....”
- Remove the Introduction and Options screens
- Remove the “Cancel button during the download phase
- Update visuals on download/install screens per https://dl.dropboxusercontent.com/u/105549/mvp.framer/index.html 
- Update messages on  download/install screens per https://dl.dropboxusercontent.com/u/105549/mvp.framer/index.html This should change every 20 seconds, disappearing for .5 seconds before cycling to the next message, returning to the first when the third is done:
---- “Fast, responsive online experiences”
----“Compatibility with more of your favorite sites”
----“Built-in privacy tools for safer browsing”

Attachments

(6 files, 11 obsolete files)

44.95 KB, patch
mhowell
: review+
Details | Diff | Splinter Review
1.49 KB, patch
mhowell
: review+
Details | Diff | Splinter Review
88.77 KB, image/png
Details
826.69 KB, image/png
Details
133.63 KB, patch
mhowell
: review+
Details | Diff | Splinter Review
53.15 KB, patch
mhowell
: review+
Details | Diff | Splinter Review
Changes to the Windows stub installer for upcoming funnelcake (bug 1322718)

- Remove the introduction and options pages
- New design for download/install UI
  - no "Cancel" button
  - no distinction between download and install
  - BG graphic
  - timed succession of headlines
- Use Firefox icon
- Change name to Firefox Installer
- Improved cancel/error message
Assignee: nobody → agashlin
Back out af88a04128f6 and 86a19f0dfa67 (bug 1305453) and 8638691b8a7a (bug 797208)
First pass at streamlining patch, expects to be applied over attachment 8823474 [details] [diff] [review]
Fix accidental debug setting
Attachment #8823475 - Attachment is obsolete: true
Once this lands let's verify that it looks correct on m-c and uplift it as soon as possible - thanks.
Flags: qe-verify+
This current patch definitely isn't in shape to be applied to m-c, I only learned that anyone was planning that today. My plan had been to use oak to build the modified stub installer alone. Additional work would be needed to make this not interfere with the preexisting stub, and I'm not sure that is even desirable for the onboarding funnelcake.

As I'm not comfortable with the workings of the flags, could you indicate this for me, lizzard?
Flags: needinfo?(lhenry)
Thanks Adam, I assumed it was part of what was needed for bug 1322718.  As far as I know, this funnelcake build is planned for the Firefox 51 release (for Jan. 23, so we want to try to land everything for it on beta as far ahead of that as possible so that we can test.)  

Dolske, gijs, is this something for a future funnelcake build or do we need it for this particular project?
Flags: needinfo?(lhenry) → needinfo?(dolske)
Updating user story field with details discussed in PRD.
User Story: (updated)
Attachment #8823474 - Attachment is obsolete: true
This includes a change to the self-extractor stub which replaces its icon with firefox64.ico, also in this patch.
Attached patch 4. Stub streamlining (obsolete) — Splinter Review
These are the changes directly to the stub installer. I hope that by separating things out I have made it clearer.

This is still waiting on a final design.
Attachment #8823483 - Attachment is obsolete: true
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #6)
> 
> Dolske, gijs, is this something for a future funnelcake build or do we need
> it for this particular project?

Hi Liz - this is definitely something for the Fx51 funnelcake -  bug 1322718.
Attachment #8823909 - Attachment is obsolete: true
Attachment #8823474 - Attachment description: Back out Win 7 restriction and 64-bit support → 1. Back out Win 7 restriction and 64-bit support
Attachment #8823474 - Attachment is obsolete: false
Attached patch 4. Stub streamlining (obsolete) — Splinter Review
Layout and timing adjustments
Attachment #8823916 - Attachment is obsolete: true
Whiteboard: [fce-active]
dolske mentioned a possible project branch and testing from that. Is that something you all are talking about with releng and QA?  
Catlee, do you know about this?  Can we do anything at the last minute to set up something for a funnelcake build?
Flags: needinfo?(catlee)
The plan is to commit this to projects/oak, we should be able to build the stub installers for the funnelcake there.

I don't know what the plans are for QA, though, there are to my knowledge no automated tests for the installer.
Attached image stub-spec.png
Final design spec.
Attached patch 4. Stub streamlining (obsolete) — Splinter Review
Attachment #8824305 - Attachment is obsolete: true
Attached file Build for testing (obsolete) —
Attached file Build for testing (fast cycle time) (obsolete) —
Both this and the other test binary (attachment 8824627 [details]):
- are not signed
- do not submit a ping on completion
- do not contain a funnelcake id

Apart from that they should behave identically to the final delivery.

This build cycles every 2 seconds instead of every 20, to show all of the messages quickly.
Attachment #8823474 - Flags: review?(mhowell)
Attachment #8824202 - Flags: review?(mhowell)
Comment on attachment 8824614 [details] [diff] [review]
3. binaries (image and icon resources, final bg)

The only change to 7zSD.sfx was to replace the icon with the newly added firefox64.ico via Resource Hacker 4.5.30. sha256 before (from ac7cef05200f) was
15a7bb2d0f08229017c97770350ee4896466d121c17be2e1029a96a61f1d01a4
and after
8cb1e4f35c4b40eafeba77baf9ca5c33e984e790ec42071490480a7e9ccb290e
Attachment #8824614 - Flags: review?(mhowell)
Attachment #8824615 - Flags: review?(mhowell)
Comment on attachment 8823474 [details] [diff] [review]
1. Back out Win 7 restriction and 64-bit support

Review of attachment 8823474 [details] [diff] [review]:
-----------------------------------------------------------------

Per comment 17, I am reviewing these patches only for landing on oak (even though pushing to oak doesn't normally require review). They should not be pushed anywhere else, at least until a final decision has been made on whether to make the streamlined installer permanent.

This patch contains no new code, it just backs out some major behavior changes to the installer since 51, because the test of this installer will be run on 51.
Attachment #8823474 - Flags: review?(mhowell) → review+
Comment on attachment 8824202 [details] [diff] [review]
2. use release branding and build the stub installer

Review of attachment 8824202 [details] [diff] [review]:
-----------------------------------------------------------------

See above re: landing these patches. This one is just config changes to force oak to build a stub installer and to use release branding.
Attachment #8824202 - Flags: review?(mhowell) → review+
Comment on attachment 8824614 [details] [diff] [review]
3. binaries (image and icon resources, final bg)

Review of attachment 8824614 [details] [diff] [review]:
-----------------------------------------------------------------

Confirmed manually that the only change to 7zSD.sfx was the icon resources, and the new images are correct.
Attachment #8824614 - Flags: review?(mhowell) → review+
Comment on attachment 8824615 [details] [diff] [review]
4. Stub streamlining

Review of attachment 8824615 [details] [diff] [review]:
-----------------------------------------------------------------

A few small-to-medium issues here, nothing catastrophic. If a few of these can't be done in time, it's not a big deal.

::: browser/branding/branding-common.mozbuild
@@ +17,5 @@
>          FINAL_TARGET_FILES.VisualElements += [
>              'VisualElements_150.png',
>              'VisualElements_70.png',
>          ]
>          BRANDING_FILES += [

Remove the files that aren't used anymore from this list (also from the one in Makefile.in, and probably also the actual files).

::: browser/installer/windows/nsis/stub.nsi
@@ +821,3 @@
>    ; Since the text color for controls is set in this Dialog the foreground and
>    ; background colors of the Dialog must also be hardcoded.
> +  SetCtlColors $Dialog 0xFFFFFF 0x0000c0

Keep these macro constants and just override them in this file, like you did with INSTALL_BLURB_TEXT_COLOR.

@@ +846,5 @@
> +!define __NSD_LabelRight_CLASS STATIC
> +!define __NSD_LabelRight_STYLE ${DEFAULT_STYLES}|${SS_NOTIFY}|${SS_RIGHT}
> +!define __NSD_LabelRight_EXSTYLE ${WS_EX_TRANSPARENT}
> +
> +  ${NSD_CreateLabelRight} -433u ${INSTALL_FOOTER_TOP_DU} 400u 20u "$(STUB_BLURB_FOOTER)"

Use nsDialogs::CreateControl instead of defining these macros just to add a style.

@@ +1261,5 @@
>      ${EndIf}
>    ${EndIf}
> +
> +  StrCpy $0 "200" ; DownloadIntervalMS is 200
> +  Call UpdateBlurb

Did you try giving UpdateBlurb its own timer, instead of having to tickle it periodically like this?

::: browser/locales/en-US/installer/nsisstrings.properties
@@ +23,2 @@
>  
>  INTRO_BLURB1=Thanks for choosing $BrandFullName, the browser that chooses you above everything else.

Go ahead and remove the strings that we're no longer using. Just need to be sure not to use any of their names.

@@ +35,5 @@
>  WARN_MIN_SUPPORTED_OSVER_MSG=Sorry, $BrandShortName can't be installed. This version of $BrandShortName requires ${MinSupportedVer} or newer. Please click the OK button for additional information.
>  WARN_MIN_SUPPORTED_CPU_MSG=Sorry, $BrandShortName can't be installed. This version of $BrandShortName requires a processor with ${MinSupportedCPU} support. Please click the OK button for additional information.
>  WARN_MIN_SUPPORTED_OSVER_CPU_MSG=Sorry, $BrandShortName can't be installed. This version of $BrandShortName requires ${MinSupportedVer} or newer and a processor with ${MinSupportedCPU} support. Please click the OK button for additional information.
>  WARN_WRITE_ACCESS=You don't have access to write to the installation directory.\n\nClick OK to select a different directory.
> +WARN_WRITE_ACCESS_QUIT=You don't have access to write to the installation directory.

The messages about the installation directory should indicate what directory we tried to use.
Attachment #8824615 - Flags: review?(mhowell) → review-
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #6)

> Dolske, gijs, is this something for a future funnelcake build or do we need
> it for this particular project?

My understanding is that this is up to Verdi and the stub installer team to coordinate with releng, since it requires a custom funnelcake build to ship a modified stub installer. Otherwise, the work in bug 1322718 is just a standard & simple repack-based funnelcake.
Flags: needinfo?(dolske)
Attached patch 4. Stub streamlining, revised (obsolete) — Splinter Review
Attachment #8824615 - Attachment is obsolete: true
Attachment #8825281 - Flags: review?(mhowell)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #16)
> dolske mentioned a possible project branch and testing from that. Is that
> something you all are talking about with releng and QA?  
> Catlee, do you know about this?  Can we do anything at the last minute to
> set up something for a funnelcake build?

(In reply to Adam Gashlin [:agashlin] from comment #17)
> The plan is to commit this to projects/oak, we should be able to build the
> stub installers for the funnelcake there.
> 
> I don't know what the plans are for QA, though, there are to my knowledge no
> automated tests for the installer.
Flags: needinfo?(catlee)
Try/Oak are a good place to do development work and initial QA. The nightly builds on Oak are signed with the same certificate used for releases, while dep builds on Oak and Try use a Mozilla-generated certificate. The stub checks the signing of the full installer it downloads so stub from try is likely to fail (see CertNameDownload, CertIssuerDownload, and URLStubDownload in dxr). Nightly builds can be started at https://secure.pub.build.mozilla.org/buildapi/self-serve/oak - you can cancel unnecessary platforms after they start - otherwise they run at 4am Pacific if the code has changed.

Once QA and yourselves are happy with Oak we'd recommend packaging the changes up to be landed on a branch in mozilla-release. I think this makes it much clearer when we look back from the future - ie that the code used was 51.0 (or 51.0.x) with a small set of modifications, rather the state of oak from <some date> plus changes. Patches 1 and 2 wouldn't be needed in m-r.

BTW, we'd would need to modify the FunnelcakeVersion variable at https://dxr.mozilla.org/mozilla-beta/source/browser/installer/windows/nsis/defines.nsi.in#11 to ensure we download the funnelcake build. Not for testing, but for the final stub.
(In reply to Nick Thomas (UTC+13) [:nthomas] from comment #33)
> The stub checks the signing of the full installer it downloads so stub from try is likely to fail

URLStubDownload is always set so the stub goes to download.mozilla.org/?os=win&lang=${AB_CD}&product=firefox${whatever}-latest to get the latest genuine signed build for a channel, so it works fine built from try. Not that we will use that beyond testing. (Am I misunderstanding your concern here?)

> Nightly builds can be started at
> https://secure.pub.build.mozilla.org/buildapi/self-serve/oak - you can
> cancel unnecessary platforms after they start - otherwise they run at 4am
> Pacific if the code has changed.

Good to know, thanks!

> Once QA and yourselves are happy with Oak we'd recommend packaging the
> changes up to be landed on a branch in mozilla-release.

This makes a lot of sense, I agree. Will you be able to help us with the branch when the time comes?

> BTW, we'd would need to modify the FunnelcakeVersion variable

Right, I think (as you've mention by email) we'll actually need two branches here, one for these patches with FunnelcakeVersion 99, the other just setting the FunnelcakeVersion to 98 for the control. Or vice versa, I've only heard about 98/99, don't know which is to be which.
(In reply to Adam Gashlin [:agashlin] from comment #34)
> URLStubDownload is always set so the stub goes to
> download.mozilla.org/?os=win&lang=${AB_CD}&product=firefox${whatever}-latest
> to get the latest genuine signed build for a channel, so it works fine built
> from try. Not that we will use that beyond testing. (Am I misunderstanding
> your concern here?)

Yes, I think you're right. I got tangled up in the cert details and forgot about the URL it'd be using.
 
> This makes a lot of sense, I agree. Will you be able to help us with the
> branch when the time comes?

Yes, no problem. To follow the one earlier case we'd use a branch name of FUNNELCAKE99_BRANCH.
 
> Right, I think (as you've mention by email) we'll actually need two branches
> here, one for these patches with FunnelcakeVersion 99, the other just
> setting the FunnelcakeVersion to 98 for the control. Or vice versa, I've
> only heard about 98/99, don't know which is to be which.

cmore has chosen 98 for the control, 99 for the experiment. For anyone following along, there's discussion elsewhere on if we need a custom stub & funnelcake full installer for the control, or can use the stub attribution project.
Changing the error/quit message as per email discussion with verdi.
Attachment #8825281 - Attachment is obsolete: true
Attachment #8825281 - Flags: review?(mhowell)
Attachment #8826316 - Flags: review?(mhowell)
Attachment #8826316 - Flags: review?(mhowell) → review+
Attachment #8824627 - Attachment is obsolete: true
Attachment #8824629 - Attachment is obsolete: true
It looks like we will be doing three different funnelcake ids, 98 as the unmodified stub, 99 and 100 as streamlined, each pulling down a differently configured full install by the funnelcake id mechanism.

As Nick suggested in comment 33 we plan on doing the final builds from a release branch after 51 is released. I wanted to run this by Release Management to see if there are any objections or preparations needed in advance. Patches 3 & 4 on this bug should be all we need besides a one-line patch to set the funnelcake id, and they apply fine against beta right now (which was expected, as patch 1 was for rolling m-c back to beta).
Flags: needinfo?(lhenry)
We just released an RC2 desktop build (on the beta channel but built from the mozilla-release repo).  So, if you were testing against builds from mozilla-beta, that is not perfectly in sync at this point with what we will be releasing. Once you have your new builds will you also be testing them or do you have testing lined up? Thanks!
Flags: needinfo?(lhenry) → needinfo?(agashlin)
Ok, I just checked and everything is fine with current mozilla-release as well, I didn't expect any surprises in the installer. I've been doing manual testing of the stub on VMs for XP, Win 7, and Win 10, and I'll do that again for the new builds.

I've seen mention of a QA testing period of one week for another aspect of the onboarding experiment
https://bugzilla.mozilla.org/show_bug.cgi?id=1326252#c7
but I'm not sure if that will include the stub as well, Chris can you answer this?
Flags: needinfo?(lhenry)
Flags: needinfo?(chrismore.bugzilla)
Flags: needinfo?(agashlin)
It should include the stub as well. QA is involved on this bug for funnelcake tests : bug 1322718
I've changed plans slightly and instead put the funnelcake id patches on separate bug 1333873.
Depends on: 1334786
No longer depends on: 1334786
Flags: needinfo?(lhenry)
Flags: needinfo?(chrismore.bugzilla)
This was landed on a project branch in bug 1326252 comment 17, builds are available for testing in bug 1326252 comment 27. All of my testing indicates that the requirements are met, if more work is needed it might make sense to open another bug for the specifics.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Depends on: 1350405
Depends on: 1366763
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [fce-active] → [fce-active-legacy]
You need to log in before you can comment on or make changes to this bug.