1097597 - Disabling Loop breaks the world

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect)

Description

[António Novo]

User Agent: Mozilla/5.0 (Windows NT 6.2; rv:36.0) Gecko/20100101 Firefox/36.0
Build ID: 20141111030203

Steps to reproduce:

Adding the following
user_pref("loop.enabled", false);
to the prefs.js on a clean/new profile (or manually setting this preference to false via the about:config on any profile).


Actual results:

The about:customize page no longer presents the user with any customization options, the page render exactly like about:blank.


Expected results:

The about:customize should continue to work without any issues after disabling Loop.

Comment 1

[:Gijs (he/him)]

This also breaks the browser console and the rest of devtools.

I have no idea how to get loop bugs prioritized, so I'm just going to needinfo someone. Mark, can you help?

Comment 2

[Mark Banner (:standard8)]

I've just done a bit of testing, this seems to affect 35 and 36 only. 34 is unaffected as far as I can tell.

On all three versions, I'm seeing errors dumped which relate to: 

console.error: 
  [CustomizableUI]
  Custom widget with id loop-button does not return a valid node

Testing on a debug nightly, I see:

JavaScript error: chrome://browser/content/browser.js, line 2248: TypeError: this.toolbarButton.node is null

just after it. Which looks like it might be this function:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-loop.js#76

Its possible that's not taking account of loop not being enabled.



I'm not convinced this is a blocker severity, as Loop is being released in 34, so we shouldn't need to disable it (or shouldn't be disabling it), and this option isn't in primary UI, hence downgrading. Obviously this could affect organisational deployments who may want to disable it and hence still a bug that we should fix.

Comment 3

[Maire Reavy [:mreavy]]

Hi Gijs, setting the "blocking-loop" flag to the release you believe this blocks is the best way to get a bug prioritized.  If it's a blocker/critical bug, doing a needinfo to me (mreavy@mozilla), Shell ([e-mail]) and/or Mark is a good idea.  I'm the eng mananger of Hello/Loop, Mark is tech lead, and Shell is our EPM.

My testing patches Mark's.  Fx34 is unaffected for me.  My testing indicates that it's also related to the throttling code -- at least for Fx35.  The throttling code in Fx34, Fx35, and fx36 is quite different.

I'm marking this as Fx35+.  Unfortunately, Mike has been out with a hand injury, but I'll needinfo him and ask Matt and/or Jaws to take a look at this.

Thanks for flagging this!

Matt -- Would you have a chance to look at this today?
Mike -- Any ideas what's going wrong?

Comment 4

[Maire Reavy [:mreavy]]

"patches" should say "matches" :-)  (My testing matches Mark's.)

Comment 5

[Matthew N. [:MattN]]

Attachment: MozReview Request: bz://1097597/MattN

Comment 6

[Matthew N. [:MattN]]

/r/497 - Bug 1097597 - Check if there is a Loop toolbarbutton node in updateToolbarState. r=Standard8

Pull down this commit:

hg pull review -r 9fe37f5753287a3d72cdbe06af68a65ffefab0b1

Comment 7

[Matthew N. [:MattN]]

This seems to work for Nightly. I couldn't reproduce just having an issue with customize mode as the rest of the browser would already be broken when about:customizing was broken.

Comment 8

[Matthew N. [:MattN]]

I would write a test for the pref change but the pref doesn't take effect until after a restart. Do you want me to simulate this state by deleting the node?

Comment 9

[Maire Reavy [:mreavy]]

(In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #7)
> This seems to work for Nightly. I couldn't reproduce just having an issue
> with customize mode as the rest of the browser would already be broken when
> about:customizing was broken.

Matt -- Can you verify that your fix fixes both Nightly (Fx36) and Aurora (Fx35)?  It's possible that the breakage is a little different since the throttling code for each is different.  Also, can you verify that your fix now allows the user to bring up the browser console on both?  (That was also failing for me due to this bug -- on both Nightly and Aurora.)  Thanks!

Comment 10

[Matthew N. [:MattN]]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #9)
> (In reply to Matthew N. [:MattN] (UTC+1 until Nov. 22) from comment #7)
> > This seems to work for Nightly. I couldn't reproduce just having an issue
> > with customize mode as the rest of the browser would already be broken when
> > about:customizing was broken.
> 
> Matt -- Can you verify that your fix fixes both Nightly (Fx36) and Aurora
> (Fx35)?

The patch applies cleanly and seems to fix Fx35 too.

> Also, can you verify that your fix
> now allows the user to bring up the browser console on both?

Yes, it does fix this.

Comment 11

[Mark Banner (:standard8)]

https://reviewboard.mozilla.org/r/495/#review239

Looks good.

Comment 12

[Matthew N. [:MattN]]

https://hg.mozilla.org/integration/fx-team/rev/af2a649a6d6b

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/af2a649a6d6b

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/555f6f98a7db

Comment 15

[Paul Silaghi, QA [:pauly]]

Verified fixed 36.0a2 (2014-12-17), 35b4 Win 7.
Note the "[CustomizableUI]" "Custom widget with id loop-button-throttled does not return a valid node" error is still present.

Comment 17

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1097597 - Check if there is a Loop toolbarbutton node in updateToolbarState. r=Standard8