876806 - Restart-less add-ons' scripts do not show up in browser debugger.

Status: VERIFIED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Girish Sharma [:Optimizer]]

Using latest nightly:
1. Install this addon for example : https://addons.mozilla.org/en-US/firefox/addon/ui-enhancer/
2. Open up Browser debugger .
3. None of the script show up. Atleast the bootstrap.js should show up.

Comment 1

[Girish Sharma [:Optimizer]]

None of its scripts*

Comment 2

[Panos Astithas (he/him) [:past] (please ni?)]

I have a fix for this.

Comment 3

[Girish Sharma [:Optimizer]]

(In reply to Panos Astithas [:past] from comment #2)
> I have a fix for this.

http://ragemaker.net/images/Happy/happy%20crying.png

Comment 4

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Properly parse jar: URLs from add-ons

uri.hostPort was throwing for jar: URLs. With this patch I can now step through the code of the add-on in comment 0.

Comment 5

[Victor Porof [:vporof][:vp]]

Comment on attachment 771284 [details] [diff] [review]
Properly parse jar: URLs from add-ons

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

::: browser/devtools/debugger/debugger-panes.js
@@ +1068,5 @@
>      }
>  
> +    let { scheme, directory, fileName } = uri;
> +    let hostPort;
> +    // Add-on SDK jar: URLs may cause this to throw.

!!

Any clue why? I don't particularly care, but it'd be nice to explain it here if you know.

@@ +1072,5 @@
> +    // Add-on SDK jar: URLs may cause this to throw.
> +    try {
> +      hostPort = uri.hostPort;
> +    } catch (e) {
> +      dumpn(e);

Could you also add the Cu.report error with an explicit message extravaganza here please?

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Attachment: Properly parse jar: URLs from add-ons

(In reply to Victor Porof [:vp] from comment #5)
> > +    // Add-on SDK jar: URLs may cause this to throw.
> 
> !!
> 
> Any clue why? I don't particularly care, but it'd be nice to explain it here
> if you know.

I dug a little deeper and this is apparently by design:
http://mxr.mozilla.org/mozilla-central/source/modules/libjar/nsJARURI.cpp#376

> @@ +1072,5 @@
> > +    // Add-on SDK jar: URLs may cause this to throw.
> > +    try {
> > +      hostPort = uri.hostPort;
> > +    } catch (e) {
> > +      dumpn(e);
> 
> Could you also add the Cu.report error with an explicit message extravaganza
> here please?

Armed with this newfound knowledge I modified the code to avoid the exception instead of catch it.

Comment 7

[Victor Porof [:vporof][:vp]]

Comment on attachment 772013 [details] [diff] [review]
Properly parse jar: URLs from add-ons

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

This is the last time I'm being surprised by things not working by design.

Comment 8

[Girish Sharma [:Optimizer]]

Can this be uplifted to Aurora (if not beta) ?

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Girish Sharma [:Optimizer] from comment #8)
> Can this be uplifted to Aurora (if not beta) ?

Yes, I'll ask for approval after the patch lands.

Comment 10

[Panos Astithas (he/him) [:past] (please ni?)]

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

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/18d601eff8b6

Comment 12

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 772013 [details] [diff] [review]
Properly parse jar: URLs from add-ons

[Approval Request Comment]
Bug caused by (feature/regressing bug #): probably bug 812083
User impact if declined: add-on developers won't be able to debug their add-ons using the debugger
Testing completed (on m-c, etc.): fx-team and m-c
Risk to taking this patch (and alternatives if risky): minor risk, this is a tiny patch to a feature targeting developers
String or IDL/UUID changes made by this patch: none

Comment 13

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 772013 [details] [diff] [review]
Properly parse jar: URLs from add-ons

[Approval Request Comment]
Bug caused by (feature/regressing bug #): probably bug 812083
User impact if declined: add-on developers won't be able to debug their add-ons using the debugger
Testing completed (on m-c, etc.): fx-team and m-c
Risk to taking this patch (and alternatives if risky): minor risk, this is a tiny patch to a feature targeting developers
String or IDL/UUID changes made by this patch: none

Comment 14

[bhavana bajaj [:bajaj]]

Comment on attachment 772013 [details] [diff] [review]
Properly parse jar: URLs from add-ons

low risk enough to land in aurora & will help developers.Approving on aurora

Comment 15

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d78e01f88bbe

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 772013 [details] [diff] [review]
Properly parse jar: URLs from add-ons

nsIURI in general does not make any garantees about the host/hostPort getters succeeding, so it's not just JAR URIs that can cause this to throw. E.g. data: URIs (nsSimpleURI) will have similar behavior.

A more robust check would be to check instanceof nsIStandardURL, rather than checking "scheme".

Comment 17

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/releases/mozilla-beta/rev/25bdd9da7c2a

Comment 18

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #16)
> nsIURI in general does not make any garantees about the host/hostPort
> getters succeeding, so it's not just JAR URIs that can cause this to throw.
> E.g. data: URIs (nsSimpleURI) will have similar behavior.
> 
> A more robust check would be to check instanceof nsIStandardURL, rather than
> checking "scheme".

Good point, filed bug 892368 for this.

Comment 19

[Ioana (away)]

How can I get the add-on scrips displayed in the debugger? I don't get them with the steps in comment 0, so I suppose there are details missing there.

Tried on Firefox 23 beta 9: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0.

Comment 20

[Girish Sharma [:Optimizer]]

I think the issue you are facing is that there are two bootstrap.js files, i.e. you have two restartless addons, and thus the browser debugger only shows one due to bug 890269 .

Comment 21

[Panos Astithas (he/him) [:past] (please ni?)]

Which add-on did you install Ioana? The one mentioned in comment 0 or another one? This one (Location Bar Enhancer) appears fine for me. Look for a pref.js or a helper.js file.

Comment 22

[Ioana (away)]

(In reply to Panos Astithas [:past] from comment #21)
> Which add-on did you install Ioana? The one mentioned in comment 0 or
> another one? This one (Location Bar Enhancer) appears fine for me. Look for
> a pref.js or a helper.js file.

That's the one I installed (the only add-on I have on this profile) and I see no scripts. Am I missing something in the steps? This is what I do:

1. Launch Firefox.
2. Install the Location Bar Enhancer add-on.
3. Press Ctrl+Shift+S.

Comment 23

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Ioana Budnar, QA [:ioana] from comment #22)
> 3. Press Ctrl+Shift+S.

That starts the content debugger. You need to launch the Browser Debugger from the Web Developer menu. Check the Enable Remote Debugging and Enable Chrome Debugging options first in the toolbox options panel.

Comment 24

[Ioana (away)]

Thanks for the help, guys :) I did reproduce the issue on Firefox 22 and verified it on 23:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0 (20130725195523)

bootstrap.js, helper.js and pref.js appear in the Firefox 23 debugger.

Comment 25

[Ioana (away)]

Verified as fixed on Firefox 24 and 25:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0