Regression in the remote debugger

RESOLVED FIXED in Firefox 32

Status

defect
P1
normal
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: marco, Assigned: ejpbruel)

Tracking

({regression})

Dependency tree / graph

Firefox Tracking Flags

(firefox30 unaffected, firefox31 unaffected, firefox32+ fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

Attachments

(1 attachment)

The error is: "ReferenceError: BrowserTabList is not defined".
The problem is most likely the same as bug 1010750.
Since bug 859372 regressed this feature, could you take this bug?
Flags: needinfo?(ejpbruel)
Sure. I'll look into this on monday. Sorry for breaking it!
Assignee: nobody → ejpbruel
Flags: needinfo?(ejpbruel)
I guess this regression is going into Aurora now?
I think we should uplift the fix as soon as it's ready.
(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?
Flags: needinfo?(mar.castelluccio)
(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 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?
(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
(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).
Flags: needinfo?(mar.castelluccio)
Posted patch Proposed fixSplinter Review
This should take care of the problem.
Attachment #8438214 - Flags: review?(past)
Attachment #8438214 - Flags: review?(mar.castelluccio)
Attachment #8438214 - Flags: review?(past) → review+
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).
Attachment #8438214 - Flags: review?(mar.castelluccio) → review+
Priority: -- → P1
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?
Whiteboard: [leave-open]
(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.
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:
Attachment #8438214 - Flags: approval-mozilla-aurora?
From bug 859372 I take it this is a regression in 32 and that 31 and above are unaffected.
Comment on attachment 8438214 [details] [diff] [review]
Proposed fix

Aurora approval granted.

Out of curiosity, why is this bug marked as leave open?
Attachment #8438214 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
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
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(ejpbruel)
Resolution: --- → FIXED
Whiteboard: [leave-open]
Target Milestone: --- → Firefox 33
Duplicate of this bug: 1013903
Flags: needinfo?(ejpbruel)
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.