Remove Firefox Hello from FF49

VERIFIED FIXED in Firefox 49

Status

Hello (Loop)
Client
VERIFIED FIXED
a year ago
2 months ago

People

(Reporter: RT, Assigned: standard8)

Tracking

unspecified
mozilla50
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 verified, firefox50 verified, relnote-firefox 49+)

Details

User Story

Requirements:
- Remove Firefox Hello from Aurora 50 and Nightly 51
- Remove Firefox Hello from Firefox in Beta 49 no later than August 1st (discussed with Sylvestre and Winston who are both OK with this). This implies that Beta users won't have a warning banner pre Hello removal, I checked with Nick and this is fine given it's a pre release channel.

Attachments

(13 attachments, 3 obsolete attachments)

7.42 MB, patch
dmose
: review+
Details | Diff | Splinter Review
4.88 KB, patch
glandium
: review+
Details | Diff | Splinter Review
4.78 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review
2.84 KB, patch
jib
: review+
Details | Diff | Splinter Review
57.72 KB, patch
standard8
: review+
Details | Diff | Splinter Review
2.83 KB, patch
mrbkap
: review+
Details | Diff | Splinter Review
800 bytes, patch
Details | Diff | Splinter Review
4.76 KB, patch
Details | Diff | Splinter Review
4.73 KB, patch
Details | Diff | Splinter Review
2.75 KB, patch
Details | Diff | Splinter Review
785 bytes, patch
Details | Diff | Splinter Review
6.53 MB, patch
Details | Diff | Splinter Review
51.80 KB, patch
Details | Diff | Splinter Review
Comment hidden (empty)
(Reporter)

Updated

a year ago
Group: mozilla-employee-confidential
(Reporter)

Updated

a year ago
Blocks: 1287826
Summary: Firefox Hello from FF49 → Remove Firefox Hello from FF49
(Reporter)

Updated

a year ago
User Story: (updated)
(Reporter)

Updated

a year ago
User Story: (updated)
(Reporter)

Updated

a year ago
User Story: (updated)
rhelmer: is there anything we need to do (as a System Add-on or otherwise in your experience) to remove Hello, besides removing the extension from the tree and some of the vestigial bits we never quite got into the add-on?
Flags: needinfo?(rhelmer)
(In reply to Ian Bicking (:ianb) from comment #1)
> rhelmer: is there anything we need to do (as a System Add-on or otherwise in
> your experience) to remove Hello, besides removing the extension from the
> tree and some of the vestigial bits we never quite got into the add-on?

Yes I think that's all you need to do.

Just removing `./browser/loop/` will remove the feature itself, but we should remove the bits in Gecko that add special exceptions for `about:loop` too (I see this in both `./media` and `./dom` right now https://dxr.mozilla.org/mozilla-central/search?q=about%3Aloop&redirect=false).

Ben, we need to ensure that we don't serve updates anymore for the Hello system add-on in future versions of Firefox so it doesn't reappear - do we need to do anything special there?
Flags: needinfo?(rhelmer) → needinfo?(bhearsum)
(In reply to Robert Helmer [:rhelmer] from comment #2)
> Just removing `./browser/loop/` will remove the feature itself, but we

Er, `./browser/extensions/loop` that is.
(In reply to Robert Helmer [:rhelmer] from comment #2)
> (In reply to Ian Bicking (:ianb) from comment #1)
> > rhelmer: is there anything we need to do (as a System Add-on or otherwise in
> > your experience) to remove Hello, besides removing the extension from the
> > tree and some of the vestigial bits we never quite got into the add-on?
> 
> Yes I think that's all you need to do.
> 
> Just removing `./browser/loop/` will remove the feature itself, but we
> should remove the bits in Gecko that add special exceptions for `about:loop`
> too (I see this in both `./media` and `./dom` right now
> https://dxr.mozilla.org/mozilla-central/
> search?q=about%3Aloop&redirect=false).
> 
> Ben, we need to ensure that we don't serve updates anymore for the Hello
> system add-on in future versions of Firefox so it doesn't reappear - do we
> need to do anything special there?

We just need to make sure it's not included in any Releases that we point live channels at. The way we're implementing the Rules right now means there's no way to add some sort of fail-safe, we'll need to rely on humans and QA doing the right thing.
Flags: needinfo?(bhearsum)
We'll want to remove the about redirector stuff too: 

https://dxr.mozilla.org/mozilla-central/source/browser/components/about/AboutRedirector.cpp#105
(In reply to Dan Mosedale (:dmose) from comment #5)
> We'll want to remove the about redirector stuff too: 
> 
> https://dxr.mozilla.org/mozilla-central/source/browser/components/about/
> AboutRedirector.cpp#105

Oh good point, this search actually turns up a few other places (including UITour):

https://dxr.mozilla.org/mozilla-central/search?q=loop&redirect=false

Also `browser/extensions/loop` is in .eslintignore, .gitignore and .hgignore
(In reply to Robert Helmer [:rhelmer] from comment #6)
> (In reply to Dan Mosedale (:dmose) from comment #5)
> > We'll want to remove the about redirector stuff too: 
> > 
> > https://dxr.mozilla.org/mozilla-central/source/browser/components/about/
> > AboutRedirector.cpp#105
> 
> Oh good point, this search actually turns up a few other places (including
> UITour):
> 
> https://dxr.mozilla.org/mozilla-central/search?q=loop&redirect=false

That link should be:
https://dxr.mozilla.org/mozilla-central/search?q=loopconversation&redirect=false
Assignee: nobody → standard8
Blocks: 1289549
Created attachment 8775182 [details] [diff] [review]
Part 1. Remove Loop code from Firefox.

This is a simple rm -r of the browser/extensions/loop directory.
Attachment #8775182 - Flags: review?(dmose)
Created attachment 8775183 [details] [diff] [review]
Part 2. Build changes for removing Hello.

These are the necessary changes to clean up the build system bits after the removals of the directory.
Attachment #8775183 - Flags: review?(mh+mozilla)
Created attachment 8775187 [details] [diff] [review]
Part 3. browser/ cleanups to remove old permissions, uitour and test code related to Loop.

This cleans up browers/ - removing the old permissions work arounds, the uitour bits relating to Loop, and fixes the tests.
Attachment #8775187 - Flags: review?(dmose)
Created attachment 8775203 [details] [diff] [review]
Part 4. Remove the special-case Loop CSP setting.

This removes the CSP work that was added for Loop in bug 1017257. Note the test that was altered in that patch has been removed in part 3.
Attachment #8775203 - Flags: review?(mrbkap)
Created attachment 8775209 [details] [diff] [review]
Part 5. Remove Loop's special permissions for about:loopconversation from MediaManager.

This cleans up the special permissions that Loop had for MediaManager. It doesn't clean up the Loop related telemetry items, as we may still be having users use Hello standalone pages for a bit - I'll file a follow-up bug for that clean up to happen once Loop has been shutdown.
Attachment #8775209 - Flags: review?(jib)
Comment on attachment 8775187 [details] [diff] [review]
Part 3. browser/ cleanups to remove old permissions, uitour and test code related to Loop.

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

::: browser/modules/webrtcUI.jsm
@@ +269,5 @@
>      host = uri.host;
>    } catch (ex) {}
>    if (!host) {
>      if (uri && uri.scheme.toLowerCase() == "about") {
> +      // For other about URIs, just use the full spec, without any #hash parts.

nit: remove the word "other" from this comment, as no about URI is special cased anymore.
Comment on attachment 8775209 [details] [diff] [review]
Part 5. Remove Loop's special permissions for about:loopconversation from MediaManager.

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

What's the value in keeping the telemetry bits?
Attachment #8775209 - Flags: review?(jib) → review+
(In reply to Jan-Ivar Bruaroey [:jib] from comment #14)
> What's the value in keeping the telemetry bits?

I was thinking mainly to keep the current LOOP_ and WEBRTC_ parts separate (until Loop is fully finished), so that it doesn't affect analysis of existing WebRTC stats should Loop (e.g. standalone web pages) get used on 50/49 during the shutdown period.
Attachment #8775203 - Flags: review?(mrbkap) → review+
Comment on attachment 8775182 [details] [diff] [review]
Part 1. Remove Loop code from Firefox.

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

rs=dmose
Attachment #8775182 - Flags: review?(dmose) → review+
Comment on attachment 8775187 [details] [diff] [review]
Part 3. browser/ cleanups to remove old permissions, uitour and test code related to Loop.

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

r=dmose with any appropriate comments addressed, and with some sort of explanation of why the obsolete-buttons change makes sense.

::: browser/components/about/AboutRedirector.cpp
@@ -257,5 @@
> -      result.AssignASCII(postfix);
> -      return NS_OK;
> -    }
> -  }
> -

This method gets removed entirely in janv's patch over in bug 1280216.  Maybe just leave the removal for that patch?

::: browser/components/customizableui/CustomizableUI.jsm
@@ -63,5 @@
>   * bumped any time a new id is added to this. Use the button id as key, and
>   * version the button is removed in as the value.  e.g. "pocket-button": 5
>   */
>  var ObsoleteBuiltinButtons = {
> -  "loop-button": 5,

It's not very clear to me why we're removing this, and the code that uses this variable doesn't shed much light either.  The loop-button is being obsoleted, correct?

::: browser/modules/webrtcUI.jsm
@@ +270,5 @@
>    } catch (ex) {}
>    if (!host) {
>      if (uri && uri.scheme.toLowerCase() == "about") {
> +      // For other about URIs, just use the full spec, without any #hash parts.
> +      host = uri.specIgnoringRef;

Might be worth adding a comment about why it's worth leaving this case.
Attachment #8775187 - Flags: review?(dmose) → review+
Attachment #8775183 - Flags: review?(mh+mozilla) → review+
What is the plan for the server part? When will the plug be pulled off? How does that fit with ESR45's lifetime?
(In reply to Mike Hommey [:glandium] from comment #18)
> What is the plan for the server part? When will the plug be pulled off? How
> does that fit with ESR45's lifetime?

Hello has never been enabled for any of the ESR branches. We don't have exact dates, but we will be monitoring usage.
(In reply to Dan Mosedale (:dmose) from comment #17)
> Comment on attachment 8775187 [details] [diff] [review]
> Part 3. browser/ cleanups to remove old permissions, uitour and test code
> related to Loop.

> ::: browser/components/customizableui/CustomizableUI.jsm
> @@ -63,5 @@
> >   * bumped any time a new id is added to this. Use the button id as key, and
> >   * version the button is removed in as the value.  e.g. "pocket-button": 5
> >   */
> >  var ObsoleteBuiltinButtons = {
> > -  "loop-button": 5,
> 
> It's not very clear to me why we're removing this, and the code that uses
> this variable doesn't shed much light either.  The loop-button is being
> obsoleted, correct?

That was necessary for the move from being built-in to being a system addon.  It did a modification so some state for CUI so that the system addon buttons would appear correctly based on the state of the prior built-in versions.  Removal of this should be fine.
(In reply to Dan Mosedale (:dmose) from comment #17)
> ::: browser/components/customizableui/CustomizableUI.jsm
> @@ -63,5 @@
> >   * bumped any time a new id is added to this. Use the button id as key, and
> >   * version the button is removed in as the value.  e.g. "pocket-button": 5
> >   */
> >  var ObsoleteBuiltinButtons = {
> > -  "loop-button": 5,
> 
> It's not very clear to me why we're removing this, and the code that uses
> this variable doesn't shed much light either.  The loop-button is being
> obsoleted, correct?

I checked with Shane who originally wrote the code - we don't need it. The button is being obsoleted, but this was more a "transition a button to an add-on" thing that actual obsolescence, essentially we shouldn't need it any more.
Created attachment 8775924 [details] [diff] [review]
Part 3. browser/ cleanups to remove old permissions, uitour and test code related to Loop.

Updated part 3 patch - addresses review comments, also removes a few more bits I found (and got review from dmose over irc for those).
Attachment #8775924 - Flags: review+
Attachment #8775187 - Attachment is obsolete: true
Group: mozilla-employee-confidential

Comment 23

a year ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/1eca699bca28
Part 1. Remove Loop code from Firefox. rs=dmose
https://hg.mozilla.org/integration/fx-team/rev/1a8114aa64c1
Part 2. Build changes for removing Loop. r=glandium
https://hg.mozilla.org/integration/fx-team/rev/e9e43e8256e1
Part 3. browser/ cleanups to remove old permissions, uitour and test code related to Loop. r=dmose
https://hg.mozilla.org/integration/fx-team/rev/82d62650cfe4
Part 4. Remove the special-case Loop CSP setting. r=mrbkap
https://hg.mozilla.org/integration/fx-team/rev/f5f1c014ca19
Part 5. Remove Loop's special permissions for about:loopconversation from MediaManager. r=jib

Comment 24

a year ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/a151859f3a20
Touch CLOBBER as Loop files have been removed. rs=bustage-fix
Unfortunately, I had to back this out for bustage like:

https://treeherder.mozilla.org/logviewer.html#?job_id=10861123&repo=fx-team#L3909

 09:35:16     INFO -  698 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/bugs/test_bug260264.html | not properly opened
 09:35:16     INFO -      checkOpened@dom/tests/mochitest/bugs/test_bug260264.html:30:32
 09:35:16     INFO -      unique_handler@dom/tests/mochitest/bugs/utils_bug260264.js:10:38
 09:35:16     INFO -      sendMouseEvent@SimpleTest/EventUtils.js:142:10
 09:35:16     INFO -      send@dom/tests/mochitest/bugs/utils_bug260264.js:12:9
 09:35:16     INFO -      run_tests8/<@dom/tests/mochitest/bugs/test_bug260264.html:168:7
 09:35:16     INFO -      Async*run_tests8@dom/tests/mochitest/bugs/test_bug260264.html:139:3
09:35:16 INFO - 699 INFO TEST-PASS | dom/tests/mochitest/bugs/test_bug260264.html | not properly blocked
Depends on: 1290517
Backout csets:

https://hg.mozilla.org/integration/fx-team/rev/27573cdada043b2c806e51f74e1cc790bd48a8b1
https://hg.mozilla.org/integration/fx-team/rev/da25267afa2979db4ef3466d4a9ac485b4359ce4
https://hg.mozilla.org/integration/fx-team/rev/5d0e37d0ee2dcee5eb41c16ef682164cffd47d33
https://hg.mozilla.org/integration/fx-team/rev/9346c1a86c6c5f09817f60eb600543bcac65ce94
https://hg.mozilla.org/integration/fx-team/rev/1eb17b0136e19cd9bfb207775fed7555e223986b
My best guess with the test failure, is that there's some wacky interaction going on with e10s mode stuff. It could be something to do with the listeners triggering stuff in the shims.

We can't really just disable Loop, as if we also set it to disabled in the tests (prefs_general.js), then the test fail as well - which implies something is wrong with the popup blocker.

I've no idea if this is a test-only thing or not.

Unfortunately I don't have much time to debug this now.

I would start off by removing things in bootstrap.js startup/shutdown, to reduce what gets set up, and then go from there.

Dan/Ed/Mike, can any of you take a look today please?
Flags: needinfo?(mconley)
Flags: needinfo?(edilee)
Flags: needinfo?(dmose)
So on my machine (opt build, Mac), the test can intermittently pass.

Starting from the current head of fx-team, with Hello in place. If I then in bootstrap.js around line 350, disable these two lines:

        window.addEventListener("unload", function () {
          Services.obs.removeObserver(_this5, "loop-status-changed");});

That's enough to make the test fail some of the time. With those lines enabled I can't get it to fail.

This really is starting to make me think of shims/e10s.
I looked into this.

Here's what I think happened:

1) The Loop system add-on was introduced
2) dom/tests/mochitest/bugs/test_bug260264.html was converted to work with e10s and enabled. There is a race in this test that waits for windows to close, but due to the way that Loop changes the speed of window closing, the test "wins" the race often, and the test passes.
3) Loop is removed. Windows are closing faster, and so now the test is losing the race way more often.

In nsGlobalWindow, we have a static counter for how many popups we've seen recently. We increment it and decrement it when popups open and close - although the decrement only occurs once the DocShell is detached, which happens asynchronously.

The test uses a utility that returns Promises for window.open and window.close, but it uses dom-window-destroyed for resolving close, which happens _before_ the counter is decremented. The dom-window-destroyed observer queues a runnable which resolves the Promise.

This means that the test is attempting to open windows before the windows from a previous test have finished detaching their DocShells (and decrementing the counter), which means that the attempts to open the window hit the popup limit, which blocks the popups.

I'm attaching a patch that switches us to using outer-window-destroyed.
Flags: needinfo?(mconley)
Created attachment 8776114 [details] [diff] [review]
Fix a race-y test_bug260264.html mochitest

In nsGlobalWindow, we have a static counter for how many popups we've seen
recently. We increment it and decrement it when popups open and close - although
the decrement only occurs once the DocShell is detached, which happens asynchronously.

The test uses a utility that returns Promises for window.open and window.close,
but it uses dom-window-destroyed for resolving close, which happens _before_ the
counter is decremented. The dom-window-destroyed observer queues a runnable which
resolves the Promise.

This means that the test is attempting to open windows before the windows from a
previous test have finished detaching their DocShells (and decrementing the counter),
which means that the attempts to open the window hit the popup limit, which blocks
the popups.

This test switches us to waiting for outer-window-destroyed instead, which gives us
a greater certainty that the decrement has occurred.

MozReview-Commit-ID: 3a7QzxelP0a
Attachment #8776114 - Flags: review?(mrbkap)
Created attachment 8776146 [details] [diff] [review]
Touch CLOBBER as Loop files have been removed, 2nd try
Attachment #8776114 - Attachment is obsolete: true
Attachment #8776114 - Flags: review?(mrbkap)
Assignee: standard8 → dmose
Status: NEW → ASSIGNED
Attachment #8776114 - Attachment is obsolete: false
Flags: needinfo?(dmose)
Attachment #8776114 - Flags: review?(mrbkap)
Thanks Mike, the patch works fine locally for me :-)
Flags: needinfo?(edilee)
Try server run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f84b9f216c2d
Comment on attachment 8776114 [details] [diff] [review]
Fix a race-y test_bug260264.html mochitest

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

::: dom/tests/mochitest/bugs/utils_bug260264.js
@@ +36,4 @@
>          promises.push((function(openedWindow) {
>            return new Promise(function(resolve) {
> +            let observer = {
> +              observe: function(subject) {

observe(subject) {
...
}

is a little nicer on the eyes (IMHO) but your call.
Attachment #8776114 - Flags: review?(mrbkap) → review+
Try server builds were fine, so I'm going to re-land this.

@mconley, I'm landing with Blake's suggested change, as it does seem nicer.

Comment 36

a year ago
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/890f2ada97ac
Fix a race-y test_bug260264.html mochitest. r=mrbkap
https://hg.mozilla.org/integration/fx-team/rev/b0c4a9c609c8
Part 1. Remove Loop code from Firefox. rs=dmose
https://hg.mozilla.org/integration/fx-team/rev/9c726a68ebc2
Part 2. Build changes for removing Loop. r=glandium
https://hg.mozilla.org/integration/fx-team/rev/d2e435e0737d
Part 3. browser/ cleanups to remove old permissions, uitour and test code related to Loop. r=dmose
https://hg.mozilla.org/integration/fx-team/rev/e85fa6a8d69a
Part 4. Remove the special-case Loop CSP setting. r=mrbkap
https://hg.mozilla.org/integration/fx-team/rev/9d637e0a4608
Part 5. Remove Loop's special permissions for about:loopconversation from MediaManager. r=jib
https://hg.mozilla.org/integration/fx-team/rev/c3565c8b1cdb
Touch CLOBBER as Loop files have been removed. rs=bustage-fix
Comment on attachment 8775182 [details] [diff] [review]
Part 1. Remove Loop code from Firefox.

Approval Request Comment
[Feature/regressing bug #]: Firefox Hello removal
[User impact if declined]: N/A
[Describe test coverage new/current, TreeHerder]: Landed in fx-team, tests are passing now.
[Risks and why]: Should be low - removing the add-on shouldn't have a big impact as it was designed to be separate. Most of the non-add-on removals are simple standalone parts that clean up easily.
[String/UUID change made/needed]: None
Attachment #8775182 - Flags: approval-mozilla-aurora?
Attachment #8775183 - Flags: approval-mozilla-aurora?
Comment on attachment 8776114 [details] [diff] [review]
Fix a race-y test_bug260264.html mochitest

Approval Request Comment
[Feature/regressing bug #]: Test only fix for a test that was semi-relying on a bad assumption and randomly failing once Hello was removed due to changing the timing of items.
[User impact if declined]: None
[Describe test coverage new/current, TreeHerder]: Run on try, landed in fx-team.
[Risks and why]: Test-only
[String/UUID change made/needed]: None
Attachment #8776114 - Flags: approval-mozilla-aurora?
Comment on attachment 8775182 [details] [diff] [review]
Part 1. Remove Loop code from Firefox.

These need new patches for Aurora - I'm working on it.
Attachment #8775182 - Flags: approval-mozilla-aurora?
Attachment #8775183 - Flags: approval-mozilla-aurora?

Comment 40

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a151859f3a20
https://hg.mozilla.org/mozilla-central/rev/890f2ada97ac
https://hg.mozilla.org/mozilla-central/rev/b0c4a9c609c8
https://hg.mozilla.org/mozilla-central/rev/9c726a68ebc2
https://hg.mozilla.org/mozilla-central/rev/d2e435e0737d
https://hg.mozilla.org/mozilla-central/rev/e85fa6a8d69a
https://hg.mozilla.org/mozilla-central/rev/9d637e0a4608
https://hg.mozilla.org/mozilla-central/rev/c3565c8b1cdb
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Created attachment 8776298 [details] [diff] [review]
Aurora patch - part 1

Approval Request Comment
[Feature/regressing bug #]: Firefox Hello removal
[User impact if declined]: N/A
[Describe test coverage new/current, TreeHerder]: Landed in fx-team, tests are passing now.
[Risks and why]: Should be low - removing the add-on shouldn't have a big impact as it was designed to be separate. Most of the non-add-on removals are simple standalone parts that clean up easily.
[String/UUID change made/needed]: None
Attachment #8776298 - Flags: approval-mozilla-aurora?
Created attachment 8776299 [details] [diff] [review]
Aurora patch - part 2
Attachment #8776299 - Flags: approval-mozilla-aurora?
Created attachment 8776300 [details] [diff] [review]
Aurora patch - part 3
Attachment #8776300 - Flags: approval-mozilla-aurora?
Created attachment 8776302 [details] [diff] [review]
Aurora patch - part 4
Attachment #8776302 - Flags: approval-mozilla-aurora?
Created attachment 8776303 [details] [diff] [review]
Aurora patch - part 5
Attachment #8776303 - Flags: approval-mozilla-aurora?
Created attachment 8776304 [details] [diff] [review]
Aurora patch - clobber fix.
Created attachment 8776306 [details] [diff] [review]
Aurora patch - part 1

Tree update issue, this is the correct part 1.
Attachment #8776298 - Attachment is obsolete: true
Attachment #8776298 - Flags: approval-mozilla-aurora?
Attachment #8776306 - Flags: approval-mozilla-aurora?
Created attachment 8776307 [details] [diff] [review]
Aurora patch - part 3 (no l10n removals)

This is part 3 without the L10n removals.
Attachment #8776300 - Attachment is obsolete: true
Attachment #8776300 - Flags: approval-mozilla-aurora?
Attachment #8776307 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 49

a year ago
bugherderuplift
Note, these were all given pre-approval by Sylvestre over email, hence I've pushed them.

https://hg.mozilla.org/releases/mozilla-aurora/rev/f3a3809acae6
https://hg.mozilla.org/releases/mozilla-aurora/rev/183d4deab3e5
https://hg.mozilla.org/releases/mozilla-aurora/rev/0b526a165010
https://hg.mozilla.org/releases/mozilla-aurora/rev/b647486109fc
https://hg.mozilla.org/releases/mozilla-aurora/rev/c85dda2fd353
https://hg.mozilla.org/releases/mozilla-aurora/rev/dcfb698a71b8
https://hg.mozilla.org/releases/mozilla-aurora/rev/77ab90179681
status-firefox49: --- → fixed
I'll leave the approval flags for tracking purposes.
Assignee: dmose → standard8
Flags: needinfo?(sledru)
Blocks: 1262447
Comment hidden (spam)
Comment on attachment 8776114 [details] [diff] [review]
Fix a race-y test_bug260264.html mochitest

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

Approve the uplift requests per comment #49 and clean the flags.
Attachment #8776114 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8776299 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8776302 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8776303 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8776306 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8776307 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(sledru)

Updated

a year ago
Depends on: 1290808
Release Note Request (optional, but appreciated)
[Why is this notable]: Obvious
[Suggested wording]: Bye bye Firefox Hello
[Links (documentation, blog post, etc)]: we will have one
relnote-firefox: --- → ?

Comment 54

a year ago
A message about the change would help, I have just throw away 1 hour looking for Hello in Firefox Devel Edition.
Btw I think that Hello is a geat tool, maybe lack the fullscreen version for normal chat. I hope it will be present as Add-on.
Cheers
(Reporter)

Comment 55

a year ago
FYI we just released the SUMO article:
https://support.mozilla.org/en-US/kb/hello-status(In reply to Sylvestre Ledru [:sylvestre] from comment #53)
> Release Note Request (optional, but appreciated)
> [Why is this notable]: Obvious
> [Suggested wording]: Bye bye Firefox Hello
> [Links (documentation, blog post, etc)]: we will have one

FYI we just released the SUMO article:
https://support.mozilla.org/en-US/kb/hello-status
(In reply to Mark Banner (:standard8) from comment #19)
> (In reply to Mike Hommey [:glandium] from comment #18)
> > What is the plan for the server part? When will the plug be pulled off? How
> > does that fit with ESR45's lifetime?
> 
> Hello has never been enabled for any of the ESR branches. We don't have
> exact dates, but we will be monitoring usage.

But it's shipped in ESR, and I'm pretty sure people have enabled it.
(In reply to Mike Hommey [:glandium] from comment #56)
> (In reply to Mark Banner (:standard8) from comment #19)
> > (In reply to Mike Hommey [:glandium] from comment #18)
> > > What is the plan for the server part? When will the plug be pulled off? How
> > > does that fit with ESR45's lifetime?
> > 
> > Hello has never been enabled for any of the ESR branches. We don't have
> > exact dates, but we will be monitoring usage.
> 
> But it's shipped in ESR, and I'm pretty sure people have enabled it.

Personally, I'd say if anything was shipped as disabled then there shouldn't be any expectations around its support. In Hello's case, we did disable it on ESR specifically because we were concerned about the support issues.

However, thanks for making us aware, we'll take a look at the numbers and discuss it with product.
Added to the 49 release notes
"Bye bye Firefox Hello"
relnote-firefox: ? → 49+

Updated

a year ago
Depends on: 1293024
Flags: qe-verify+
Depends on: 1294540
Blocks: 1295169
Blocks: 1295606
I can confirm that Hello has been disabled on Firefox 49 beta 8 across platforms (Windows 10 x64, Ubuntu 14.04 x64 and Mac OS X 10.10.5).
status-firefox49: fixed → verified
Flags: qe-verify+
Comment hidden (spam)
Assuming the user story associated to this bug is still standing, we should make sure Firefox Hello has been successfully removed from Firefox 50 and 51 as well.

Bogdan, could you please take a look at this?
Flags: needinfo?(bogdan.maris)
(Reporter)

Comment 62

10 months ago
(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #61)
> Assuming the user story associated to this bug is still standing

Yes it is, Hello should be out of all post 49 releases

> make sure Firefox Hello has been successfully removed from Firefox 50 and 51
> as well.
> 
> Bogdan, could you please take a look at this?
(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #61)
> Assuming the user story associated to this bug is still standing, we should
> make sure Firefox Hello has been successfully removed from Firefox 50 and 51
> as well.
> 
> Bogdan, could you please take a look at this?

(In reply to Romain Testard [:RT] from comment #62)
> (In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #61)
> > Assuming the user story associated to this bug is still standing
> 
> Yes it is, Hello should be out of all post 49 releases
> 
> > make sure Firefox Hello has been successfully removed from Firefox 50 and 51
> > as well.
> > 
> > Bogdan, could you please take a look at this?

Confirming that it's out of 50 and 51 as well across platforms (Windows 10 64bit, Mac OS X 10.10.5 and Ubuntu 16.04 32bit).
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
Flags: needinfo?(bogdan.maris)
Depends on: 1343830
Comment hidden (spam)
You need to log in before you can comment on or make changes to this bug.