1165003 - Warnings generated if Loop is disabled (e.g. on ESR)

Status: RESOLVED DUPLICATE of bug 1108548

(Hello (Loop) :: Client, defect, P5)

Description

[Mike Kaply [:mkaply]]

Hello was disabled on the ESR, but not completely as errors show up in the console:

"[CustomizableUI]" "Custom widget with id loop-button does not return a valid node"


and

A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

Date: Thu May 14 2015 15:13:18 GMT-0500 (Central Standard Time)
Full Message: Error: loop is not enabled
Full Stack: this.MozLoopService.initialize<@resource:///modules/loop/MozLoopService.jsm:1110:29
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
LoopUI.init@chrome://browser/content/browser.js:4442:7
gBrowserInit._delayedStartup@chrome://browser/content/browser.js:12083:5


Hello should be disabled properly so no errors show up on the console.

Comment 1

[:shell escalante]

hi guys - wanted to ask priority/impact/risk on fixing this bug because timing wise with ESR stuff (and we won't stand-up again until Monday - wasn't sure how time sensitive)

Comment 2

[Mike de Boer [:mikedeboer]]

Not critical, very low prio. The errors mentioned are only expected errors that are logged to the console. No code path is broken that would lead to a downgraded experience.

Comment 3

[Frédéric Wang (:fredw)]

Any suggestion about the way to fix this bug?

FYI, this is causing issue with the testing feature of the addon sdk (I know, it is no longer maintained...). The logged errors make "jpm test" fail. Passing a JSON config with "loop.enabled" set to true does not seem to help...

Comment 4

[Frédéric Wang (:fredw)]

(In reply to Frédéric Wang (:fredw) from comment #3)
> FYI, this is causing issue with the testing feature of the addon sdk (I
> know, it is no longer maintained...). The logged errors make "jpm test"
> fail. Passing a JSON config with "loop.enabled" set to true does not seem to
> help...

I forgot to say that this is similar to bug 1103385...

Comment 5

[Mike de Boer [:mikedeboer]]

Well, the solution would be to remove the code that yields the warning in MozLoopService.jsm.

Comment 6

[Frédéric Wang (:fredw)]

(In reply to Mike de Boer [:mikedeboer] from comment #5)
> Well, the solution would be to remove the code that yields the warning in
> MozLoopService.jsm.

So if I read the code correctly, the changes to browser/base/content/browser-loop.js made in bug 1157712 will avoid the logging of the "loop is not enabled" error in Gecko 41. So here we "only" need to make a patch for the ESR branch, right?

Comment 7

[Mike de Boer [:mikedeboer]]

(In reply to Frédéric Wang (:fredw) from comment #6)
> So here we "only"
> need to make a patch for the ESR branch, right?

Indeed!

Comment 8

[Mike Kaply [:mkaply]]

That patch should be for the base code as well.

There are enterprises that disable WebRTC on the mainline as well and we don't want to see that error either.

We don't want to do ESR specific patches unless absolutely necessary.

Comment 9

[Frédéric Wang (:fredw)]

(In reply to Mike Kaply [:mkaply] from comment #8)
> That patch should be for the base code as well.
> 
> There are enterprises that disable WebRTC on the mainline as well and we
> don't want to see that error either.
> 
> We don't want to do ESR specific patches unless absolutely necessary.

As I said in comment 6, the "loop is not enabled" error seems to be fixed in Gecko 41, so we can just port that fix to ESR.

However, I only read the code and didn't try to debug, so I'm not sure yet in which circumstances the "Custom widget with id loop-button does not return a valid node" is logged (and whether it is actually fixed):
https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser-loop.js?case=true&from=browser-loop.js#20

Do you mean that you still see the errors on mozilla-central?

Comment 10

[Mike Kaply [:mkaply]]

Sorry, I missed comment 6.

I see no reason to put this on ESR 38 for this specific fix. If it is fixed in 41, we can close this.

Comment 11

[Mike Kaply [:mkaply]]

I still get this error:

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

Either loop needs to catch this error and ignore (if possible) or CustomizableUI needs to stop putting out errors that can't be caught.

Comment 12

[Mark Banner (:standard8)]

(In reply to Frédéric Wang (:fredw) from comment #9)
> However, I only read the code and didn't try to debug, so I'm not sure yet
> in which circumstances the "Custom widget with id loop-button does not
> return a valid node" is logged (and whether it is actually fixed):
> https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser-
> loop.js?case=true&from=browser-loop.js#20

I suspect the error is still present, and it is probably reproducible only if Loop is disabled via the loop.enabled pref.

If you look at CustomizableWidgets.jsm you'll see that the button isn't generated if the pref is false.

Comment 13

[ISHIKAWA, Chiaki]

Hi,

The following message now shows up during the run of TB ( 38.3.0 under linux 64-bit)

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

I think it only began showing up in the last several weeks. (Not that I checked  
the .xsession-errors file all the time: this file records the console messages written by
X-based applications invoked from desktop menus under Debian desktop (Debian GNU/Linux).
Still, I don't think I have seen this particular message before.

Comment 14

[Mark Banner (:standard8)]

(In reply to ISHIKAWA, Chiaki from comment #13)
> console.error:
>   [CustomizableUI]
>   Custom widget with id loop-button does not return a valid node

Unless Thunderbird or some other add-on has started pulling in files from browser/ (i.e. CustomizableUI/CustomizableWidget) this shouldn't be possible.

I've done a quick search in mxr of comm-central and there's no references in TB specific code to these files. Therefore, I can only conclude that its some add-on that is causing the message.

Comment 15

[ISHIKAWA, Chiaki]

(In reply to Mark Banner (:standard8) from comment #14)
> (In reply to ISHIKAWA, Chiaki from comment #13)
> > console.error:
> >   [CustomizableUI]
> >   Custom widget with id loop-button does not return a valid node
> 
> Unless Thunderbird or some other add-on has started pulling in files from
> browser/ (i.e. CustomizableUI/CustomizableWidget) this shouldn't be possible.
> 
> I've done a quick search in mxr of comm-central and there's no references in
> TB specific code to these files. Therefore, I can only conclude that its
> some add-on that is causing the message.

I will check if TB add-on may cause this to be printed.
(Come to think of it, this message is not prefixed by any process message, now I have to check if this comes from iceweasel (Debian's own firefox version). 

TIA

Comment 16

[ISHIKAWA, Chiaki]

(In reply to ISHIKAWA, Chiaki from comment #15)
> (In reply to Mark Banner (:standard8) from comment #14)
> > (In reply to ISHIKAWA, Chiaki from comment #13)
> > > console.error:
> > >   [CustomizableUI]
> > >   Custom widget with id loop-button does not return a valid node
> > 
> > Unless Thunderbird or some other add-on has started pulling in files from
> > browser/ (i.e. CustomizableUI/CustomizableWidget) this shouldn't be possible.
> > 
> > I've done a quick search in mxr of comm-central and there's no references in
> > TB specific code to these files. Therefore, I can only conclude that its
> > some add-on that is causing the message.
> 
> I will check if TB add-on may cause this to be printed.
> (Come to think of it, this message is not prefixed by any process message,
> now I have to check if this comes from iceweasel (Debian's own firefox
> version). 
> 
> TIA

Sorry for the confusion.
Indeed, the message came from iceweasel (Debian's re-branding of firefox), and not from TB.

Comment 17

[Mark Banner (:standard8)]

This is actually a basic duplicate of bug 1108548. However, I'll also note that bug is now WFM as the issue has been fixed in the latest 45 series, due to changes in the Hello architecture. Therefore its time to close this bug.