Remove Firefox Hello from FF49

VERIFIED FIXED in Firefox 49

Status

Hello (Loop)
Client
VERIFIED FIXED
10 months ago
13 days 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

10 months ago
Group: mozilla-employee-confidential
(Reporter)

Updated

10 months ago
Blocks: 1287826
Summary: Firefox Hello from FF49 → Remove Firefox Hello from FF49
(Reporter)

Updated

10 months ago
User Story: (updated)
(Reporter)

Updated

10 months ago
User Story: (updated)
(Reporter)

Updated

10 months ago
User Story: (updated)

Comment 1

10 months ago
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.

Comment 4

10 months ago
(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)

Comment 5

10 months ago
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)

Updated

10 months ago
Assignee: nobody → standard8
Blocks: 1289549
(Assignee)

Comment 8

10 months ago
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)
(Assignee)

Comment 9

10 months ago
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)
(Assignee)

Comment 10

10 months ago
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)
(Assignee)

Comment 11

10 months ago
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)
(Assignee)

Comment 12

10 months ago
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+
(Assignee)

Comment 15

10 months ago
(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.

Updated

10 months ago
Attachment #8775203 - Flags: review?(mrbkap) → review+

Comment 16

10 months ago
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 17

10 months ago
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+

Updated

10 months ago
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?
(Assignee)

Comment 19

10 months ago
(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.
(Assignee)

Comment 21

10 months ago
(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.
(Assignee)

Comment 22

10 months ago
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+
(Assignee)

Updated

10 months ago
Attachment #8775187 - Attachment is obsolete: true
(Assignee)

Updated

10 months ago
Group: mozilla-employee-confidential

Comment 23

10 months 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

10 months 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
(Assignee)

Comment 25

10 months ago
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
(Assignee)

Updated

10 months ago
Depends on: 1290517
(Assignee)

Comment 26

10 months ago
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
(Assignee)

Comment 27

10 months ago
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)
(Assignee)

Comment 28

10 months ago
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.

Comment 29

10 months ago
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)

Comment 30

10 months ago
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

Updated

10 months ago
Attachment #8776114 - Flags: review?(mrbkap)

Comment 31

10 months ago
Created attachment 8776146 [details] [diff] [review]
Touch CLOBBER as Loop files have been removed, 2nd try

Updated

10 months ago
Attachment #8776114 - Attachment is obsolete: true
Attachment #8776114 - Flags: review?(mrbkap)

Updated

10 months ago
Assignee: standard8 → dmose
Status: NEW → ASSIGNED

Updated

10 months ago
Attachment #8776114 - Attachment is obsolete: false
Flags: needinfo?(dmose)
Attachment #8776114 - Flags: review?(mrbkap)
(Assignee)

Comment 32

10 months ago
Thanks Mike, the patch works fine locally for me :-)
Flags: needinfo?(edilee)

Comment 33

10 months ago
Try server run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f84b9f216c2d

Comment 34

10 months ago
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+
(Assignee)

Comment 35

10 months ago
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

10 months 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
(Assignee)

Comment 37

10 months ago
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?
(Assignee)

Updated

10 months ago
Attachment #8775183 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 38

10 months ago
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?
(Assignee)

Comment 39

10 months ago
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?
(Assignee)

Updated

10 months ago
Attachment #8775183 - Flags: approval-mozilla-aurora?

Comment 40

10 months 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: 10 months ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(Assignee)

Comment 41

10 months ago
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?
(Assignee)

Comment 42

10 months ago
Created attachment 8776299 [details] [diff] [review]
Aurora patch - part 2
Attachment #8776299 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 43

10 months ago
Created attachment 8776300 [details] [diff] [review]
Aurora patch - part 3
Attachment #8776300 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 44

10 months ago
Created attachment 8776302 [details] [diff] [review]
Aurora patch - part 4
Attachment #8776302 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 45

10 months ago
Created attachment 8776303 [details] [diff] [review]
Aurora patch - part 5
Attachment #8776303 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 46

10 months ago
Created attachment 8776304 [details] [diff] [review]
Aurora patch - clobber fix.
(Assignee)

Comment 47

10 months ago
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?
(Assignee)

Comment 48

10 months ago
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

10 months 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
(Assignee)

Comment 50

10 months ago
I'll leave the approval flags for tracking purposes.
Assignee: dmose → standard8
Flags: needinfo?(sledru)
(Assignee)

Updated

10 months ago
Blocks: 1262447
Comment hidden (spam)

Comment 52

10 months ago
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+

Updated

10 months ago
Attachment #8776299 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

10 months ago
Attachment #8776302 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

10 months ago
Attachment #8776303 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

10 months ago
Attachment #8776306 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

10 months ago
Attachment #8776307 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

10 months ago
Flags: needinfo?(sledru)

Updated

10 months 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

10 months 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

10 months 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.
(Assignee)

Comment 57

10 months ago
(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

10 months ago
Depends on: 1293024
Flags: qe-verify+
Depends on: 1294540
(Assignee)

Updated

9 months ago
Blocks: 1295169
(Assignee)

Updated

9 months ago
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

8 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.