Marco Castelluccio [:marco] Reporter
	Description • 10 years ago

The error is: "ReferenceError: BrowserTabList is not defined".
The problem is most likely the same as bug 1010750.

	Marco Castelluccio [:marco] Reporter
	Comment 1 • 10 years ago

Since bug 859372 regressed this feature, could you take this bug?

	Eddy Bruel [:ejpbruel] Assignee
	Comment 2 • 10 years ago

Sure. I'll look into this on monday. Sorry for breaking it!

	Marco Castelluccio [:marco] Reporter
	Comment 3 • 10 years ago

I guess this regression is going into Aurora now?
I think we should uplift the fix as soon as it's ready.

	Eddy Bruel [:ejpbruel] Assignee
	Comment 4 • 10 years ago

(In reply to Marco Castelluccio [:marco] from comment #3)
> I guess this regression is going into Aurora now?
> I think we should uplift the fix as soon as it's ready.

From what I can tell, this bug is a duplicate of bug 1010750, and should have been fixed when that bug got fixed.

Marco, can you confirm this for me?

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 5 • 10 years ago

(In reply to Eddy Bruel [:ejpbruel] from comment #4)
> (In reply to Marco Castelluccio [:marco] from comment #3)
> > I guess this regression is going into Aurora now?
> > I think we should uplift the fix as soon as it's ready.
> 
> From what I can tell, this bug is a duplicate of bug 1010750, and should
> have been fixed when that bug got fixed.
> 
> Marco, can you confirm this for me?

I haven't tested, but I don't believe it's fixed...  The WebappRT code path[1] likely needs similar fixes as were needed for Fennec in bug 1010750.

[1]: http://dxr.mozilla.org/mozilla-central/source/webapprt/RemoteDebugger.jsm#22

	Eddy Bruel [:ejpbruel] Assignee
	Comment 6 • 10 years ago

(In reply to J. Ryan Stinnett [:jryans] from comment #5)
> (In reply to Eddy Bruel [:ejpbruel] from comment #4)
> > (In reply to Marco Castelluccio [:marco] from comment #3)
> > > I guess this regression is going into Aurora now?
> > > I think we should uplift the fix as soon as it's ready.
> > 
> > From what I can tell, this bug is a duplicate of bug 1010750, and should
> > have been fixed when that bug got fixed.
> > 
> > Marco, can you confirm this for me?
> 
> I haven't tested, but I don't believe it's fixed...  The WebappRT code
> path[1] likely needs similar fixes as were needed for Fennec in bug 1010750.
> 
> [1]:
> http://dxr.mozilla.org/mozilla-central/source/webapprt/RemoteDebugger.jsm#22

In that case, how can I test this code path?

	J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)
	Comment 7 • 10 years ago

(In reply to Eddy Bruel [:ejpbruel] from comment #6)
> (In reply to J. Ryan Stinnett [:jryans] from comment #5)
> > [1]:
> > http://dxr.mozilla.org/mozilla-central/source/webapprt/RemoteDebugger.jsm#22
> 
> In that case, how can I test this code path?

At the moment, it's a little tricky...  You'd need to install some web app from the marketplace into desktop Firefox, and then follow Myk's steps[1].

Eventually, these apps will be shown in the App Manager UI, but for now it's a bit tricky to do.

Marco or Myk may have better / more up to date advice.

[1]: https://hacks.mozilla.org/2014/03/better-integration-for-open-web-apps-on-android/comment-page-1/#comment-2159140

	Marco Castelluccio [:marco] Reporter
	Comment 8 • 10 years ago

(In reply to Eddy Bruel [:ejpbruel] from comment #6)
> (In reply to J. Ryan Stinnett [:jryans] from comment #5)
> > (In reply to Eddy Bruel [:ejpbruel] from comment #4)
> > > (In reply to Marco Castelluccio [:marco] from comment #3)
> > > > I guess this regression is going into Aurora now?
> > > > I think we should uplift the fix as soon as it's ready.
> > > 
> > > From what I can tell, this bug is a duplicate of bug 1010750, and should
> > > have been fixed when that bug got fixed.
> > > 
> > > Marco, can you confirm this for me?
> > 
> > I haven't tested, but I don't believe it's fixed...  The WebappRT code
> > path[1] likely needs similar fixes as were needed for Fennec in bug 1010750.
> > 
> > [1]:
> > http://dxr.mozilla.org/mozilla-central/source/webapprt/RemoteDebugger.jsm#22
> 
> In that case, how can I test this code path?

./mach webapprt-test-chrome

There's a browser_debugger.js test, but you can't run it individually (bug 996259).

	Eddy Bruel [:ejpbruel] Assignee
	Comment 9 • 10 years ago

This should take care of the problem.

	Marco Castelluccio [:marco] Reporter
	Comment 10 • 10 years ago

Comment on attachment 8438214 [details] [diff] [review]
Proposed fix

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

It would be great if you could also test manually to install an app
and launch it in debug mode (as explained in the link Ryan suggested
or at https://developer.mozilla.org/en-US/Marketplace/Options/Open_web_apps_for_desktop#Can_I_debug_my_apps_when_they_are_running_on_a_desktop.3F),
as the test doesn't cover all the code (even if I think it does cover
enough).

	Eddy Bruel [:ejpbruel] Assignee
	Comment 11 • 10 years ago

I've tested the fix locally as requested and it works for me.

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

Does this need an aurora uplift as well?

	Marco Castelluccio [:marco] Reporter
	Comment 12 • 10 years ago

(In reply to Eddy Bruel [:ejpbruel] from comment #11)
> I've tested the fix locally as requested and it works for me.
> 
> https://hg.mozilla.org/integration/fx-team/rev/bd63a8720a1b
> 
> Does this need an aurora uplift as well?

It would be great if we could uplift to Aurora.

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 10 years ago

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

	Eddy Bruel [:ejpbruel] Assignee
	Comment 14 • 10 years ago

Comment on attachment 8438214 [details] [diff] [review]
Proposed fix

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 859372
User impact if declined: Using the remote debugger to debug apps won't work
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Marginal. The only changes involved are mechanical in nature.
String or IDL/UUID changes made by this patch:

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 15 • 10 years ago

From bug 859372 I take it this is a regression in 32 and that 31 and above are unaffected.

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 16 • 10 years ago

Comment on attachment 8438214 [details] [diff] [review]
Proposed fix

Aurora approval granted.

Out of curiosity, why is this bug marked as leave open?

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 10 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/006afc4e1220

(In reply to Lawrence Mandel [:lmandel] from comment #16)
> Out of curiosity, why is this bug marked as leave open?

Going off the assumption that it was for the branch uplift. Eddy, note that bug resolution tracks landing on trunk, so leaving the bug open for branch uplifts only confuses things. That said, if I got that wrong, please reopen and whip me furiously for my insolence :P