Closed Bug 1098559 Opened 5 years ago Closed 5 years ago

Install Firefox 33.1.1 from hotfix

Categories

(Firefox :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: spohl, Assigned: spohl)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 3 obsolete files)

+++ This bug was initially created as a clone of Bug #1072615 +++

+++ This bug was initially created as a clone of Bug #1061927 +++

We should update the hotfix to upgrade to 33.1
Attached patch Patch (obsolete) — Splinter Review
This mirrors the patch from bug 1072615.
Attachment #8522491 - Flags: review?(robert.strong.bugs)
We should actually use Firefox 33.1.1, which fixes a startup crash for users upgrading from Firefox 32 and below.
Sorry, where's 33.1.1? I just asked about this yesterday on #developers and no-one seemed to be aware of a 33.1.1...
I believe it got go to build today, it will be put live tomorrow AFAIK
*cries*
Bug 1021265 is the fix included in 33.1.1 BTW
Comment on attachment 8522491 [details] [diff] [review]
Patch

Bah... needs to be updated for 31.1.1 :(
Attachment #8522491 - Flags: review?(robert.strong.bugs)
Attached patch PatchSplinter Review
Attachment #8522491 - Attachment is obsolete: true
Attachment #8524089 - Flags: review?(robert.strong.bugs)
Summary: Install Firefox 33.1 from hotfix → Install Firefox 33.1.1 from hotfix
Don't apply hotfix if app.update.url.override starts with https://update.commbiz.commbank.com.au (see bug 928173 comment 32).
Attachment #8524091 - Flags: review?(robert.strong.bugs)
Attachment #8524089 - Flags: review?(robert.strong.bugs) → review+
Comment on attachment 8524091 [details] [diff] [review]
Respect app.update.url.override for commbank

>From: Stephen Pohl <spohl.mozilla.bugs@gmail.com>
>
>Bug 1098559: Don't apply hotfix if app.update.url.override starts with https://update.commbiz.commbank.com.au (see bug 928173 comment 32). r=rstrong
>
>diff --git a/v20140527.01/resource/update.jsm b/v20140527.01/resource/update.jsm
>--- a/v20140527.01/resource/update.jsm
>+++ b/v20140527.01/resource/update.jsm
>@@ -1009,16 +1009,28 @@ UpgradeManager.prototype = {
>       }
> 
>       if (!Services.prefs.getBoolPref("app.update.auto")) {
>         this._log.warn("Not applicable - app.update.auto disabled");
>         return false;
>       }
>     } catch (e) {}
> 
>+    // Don't install if app.update.url.override starts with
>+    // https://update.commbiz.commbank.com.au (see bug 928173 comment 32).
>+    try {
>+      let overrideUrl = Services.prefs.getCharPref("app.update.url.override");
>+      if (overrideUrl.startsWith("https://update.commbiz.commbank.com.au")) {
>+        this._log.warn("Not applicable - app.update.url.override starts with " +
>+                       "https://update.commbiz.commbank.com.au (bug 928173 " +
>+                        "comment 32)");
nit: alignment

>+        return false;
>+      }
>+    } catch (e) {}
>+
>     this._log.warn("Hotfix applicable");
>     return true;
>   },
> 
>   /**
>    * Whether it appears we have upgraded to the version we were supposed to.
>    */
>   _isUpgraded: function () {
Attachment #8524091 - Flags: review?(robert.strong.bugs) → review+
Good eye! :-) Fixed alignment, carrying over r+.

Thanks, Robert!
Attachment #8524091 - Attachment is obsolete: true
Attachment #8524096 - Flags: review+
Paul, just a heads up that we're planning to release a new hotfix with a workaround in place like you described in bug 928173 comment 40.
Thanks all. Much appreciated.
Comment on attachment 8524096 [details] [diff] [review]
Respect app.update.url.override for commbank

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

::: v20140527.01/resource/update.jsm
@@ +1017,5 @@
> +    // Don't install if app.update.url.override starts with
> +    // https://update.commbiz.commbank.com.au (see bug 928173 comment 32).
> +    try {
> +      let overrideUrl = Services.prefs.getCharPref("app.update.url.override");
> +      if (overrideUrl.startsWith("https://update.commbiz.commbank.com.au")) {

Argh. String.prototype.startsWith() is experimental and only implemented in FF17+[1]. Since we want FF 14.0.1 in particular to perform this check, we'll need to change this to indexOf(). Will post followup shortly.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/startsWith
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8528439 - Flags: review?(robert.strong.bugs)
This was supposed to check for '== 0'.
Attachment #8528439 - Attachment is obsolete: true
Attachment #8528439 - Flags: review?(robert.strong.bugs)
Attachment #8528441 - Flags: review?(robert.strong.bugs)
Attachment #8528441 - Flags: review?(robert.strong.bugs) → review+
http://hg.mozilla.org/releases/firefox-hotfixes/rev/56c7c4faa8b7
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Somewhere along the way we started linking directly to the installer on ftp.mozilla.org. Any updates thereafter maintained this mistake. We should link to the bouncer instead.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
:gps is handling this in bug 1107156.
Blocks: 1107156
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.