Closed Bug 1258892 Opened 4 years ago Closed 4 years ago

Cannot inspect imported scripts in worker

Categories

(DevTools :: Debugger, defect, P1)

48 Branch
x86
macOS
defect

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: arcturus, Assigned: jlast)

References

Details

Attachments

(1 file, 1 obsolete file)

While using new feature for debugging serviceworkers in FF 48, I can debug perfectly the installed worker.

But if that worker perform any importScript, the files appear in the sources panel, but content looks pretty much like this:

300: file:///
200: filename content-length last-modified file-type
201: .DS_Store 6148 Mon,%2025%20Jan%202016%2020:52:28%20GMT FILE 
201: .DocumentRevisions-V100 0 Thu,%2010%20Mar%202016%2013:55:15%20GMT DIRECTORY 
201: .OSInstallerMessages 1489 Sun,%2027%20Sep%202015%2019:25:29%20GMT FILE 
201: .Spotlight-V100 0 Mon,%2011%20Jul%202011%2018:39:57%20GMT DIRECTORY 
201: .Trashes 0 Sun,%2027%20Sep%202015%2019:28:41%20GMT DIRECTORY 
201: .dbfseventsd 0 Fri,%2006%20Jun%202014%2007:47:00%20GMT 
201: .file 0 Mon,%2013%20Jul%202015%2005:38:08%20GMT FILE 
201: .fseventsd 0 Tue,%2022%20Mar%202016%2022:41:32%20GMT DIRECTORY 
201: .hotfiles.btree 983040 Sun,%2010%20Jan%202016%2021:50:21%20GMT FILE 
201: .vol 0 Tue,%2009%20Sep%202014%2023:27:56%20GMT DIRECTORY 
201: Applications 0 Tue,%2022%20Mar%202016%2015:00:20%20GMT DIRECTORY 
201: Library 0 Mon,%2028%20Sep%202015%2009:25:35%20GMT DIRECTORY 
201: Network 0 Mon,%2013%20Jul%202015%2005:37:59%20GMT DIRECTORY 
201: System 0 Sun,%2027%20Sep%202015%2018:42:53%20GMT DIRECTORY 
201: User%20Guides%20And%20Information 0 Sun,%2027%20Sep%202015%2019:08:10%20GMT DIRECTORY 
201: Users 0 Tue,%2029%20Sep%202015%2013:39:16%20GMT DIRECTORY 
201: Volumes 0 Tue,%2022%20Mar%202016%2020:05:23%20GMT DIRECTORY 
201: bin 0 Sun,%2027%20Sep%202015%2018:44:01%20GMT DIRECTORY 
201: cores 0 Mon,%2013%20Jul%202015%2005:37:57%20GMT DIRECTORY 
201: data 0 Sat,%2025%20Aug%202012%2018:56:46%20GMT DIRECTORY 
201: dev 0 Thu,%2010%20Mar%202016%2013:53:25%20GMT DIRECTORY 
201: etc 0 Tue,%2022%20Mar%202016%2000:07:07%20GMT DIRECTORY 
201: home 0 Tue,%2022%20Mar%202016%2022:31:03%20GMT DIRECTORY 
201: installer.failurerequests 313 Sat,%2025%20Jul%202015%2004:37:49%20GMT FILE 
201: net 0 Tue,%2022%20Mar%202016%2022:31:03%20GMT DIRECTORY 
201: opt 0 Sun,%2013%20Dec%202015%2014:29:46%20GMT DIRECTORY 
201: private 0 Sun,%2027%20Sep%202015%2018:53:16%20GMT DIRECTORY 
201: sbin 0 Sun,%2027%20Sep%202015%2019:18:42%20GMT DIRECTORY 
201: tmp 0 Tue,%2022%20Mar%202016%2020:42:19%20GMT DIRECTORY 
201: usr 0 Mon,%2028%20Sep%202015%2009:25:42%20GMT DIRECTORY 
201: var 0 Sun,%2027%20Sep%202015%2019:23:22%20GMT DIRECTORY
Well, that isn't very good now, is it?

Making (service) worker debugging not painful to use is on our radar for Q1, so this should be fixed asap. Marking this as P1.
Priority: -- → P1
Assignee: nobody → jlaster
Jason, any progress on this yet?

No huge rush, just wanted to know if you need any help.
None yet, will hopefully get to it soon.
Attached patch import.1.patch (obsolete) — Splinter Review
The patch fixes the problem by getting a contentType for the source.

The patch assumes that the URL will just be the pathname because workers are restricted to the same domain. If this assumption does not hold, we could do something simple to parse the pathname out of the url.
Attachment #8740001 - Flags: review?(ejpbruel)
Comment on attachment 8740001 [details] [diff] [review]
import.1.patch

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

This looks good to me Jason. Go ahead and land it!

::: devtools/server/actors/utils/TabSources.js
@@ +325,5 @@
>              }
>              else if (extension === "js") {
>                spec.contentType = "text/javascript";
>              }
>            } catch (e) {

Please add a comment here explaining why this code is necessary. What fails if this code isn't here? Why does it fail? How does this code fix that?

I've also been thinking that we should probably start referring to bug numbers in these comments whenever we fix something. Bugzilla often contains much more information about a bug than can be contained in a comment. What do you think?
Attachment #8740001 - Flags: review?(ejpbruel) → review+
Attached patch import.2.patchSplinter Review
Attachment #8740001 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/15d2b8a78f3b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Blocks: 1273097
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.