Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Fix including non-JS scripts from HTML source#4684

Merged
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
ryanjduffy:feature/3307
Nov 14, 2017
Merged

Fix including non-JS scripts from HTML source#4684
jasonLaster merged 1 commit into
firefox-devtools:masterfrom
ryanjduffy:feature/3307

Conversation

@ryanjduffy

@ryanjduffy ryanjduffy commented Nov 13, 2017 •
edited by jasonLaster
Loading

Copy link
Copy Markdown
Contributor

Associated Issue: #3307

Summary of Changes

Updates the dependency to parse-script-tags to version 0.1.4 which supports omitting script tags that have non-JS types.

Test Plan

  • Added breakpoints on a Wordpress site (http://davidwalsh.name) with non-JS <script>s
  • Created a simple sample with non-JS <script>:
<html>
<script type="application/ld+json">
  var notScript = true;
  alert(notScript);
</script>
<script>
  console.log('from script');
</script>
<body>
Not Script
</body>
</html>

@ryanjduffy ryanjduffy mentioned this pull request Nov 13, 2017
Closed
@jasonLaster jasonLaster merged commit b98fa76 into firefox-devtools:master Nov 14, 2017
@ryanjduffy ryanjduffy deleted the feature/3307 branch November 14, 2017 02:12
@jasonLaster

jasonLaster commented Nov 21, 2017

Copy link
Copy Markdown
Contributor

screen shot 2017-11-21 at 9 50 18 am

codehag added a commit to codehag/debugger.html that referenced this pull request Dec 1, 2017
@codehag
Revert "Fix including non-JS scripts from HTML source (firefox-devtoo…
efa4ca3 
…ls#4684)"

This reverts commit b98fa76.
codehag added a commit to codehag/debugger.html that referenced this pull request Dec 1, 2017
@codehag
Revert "Fix including non-JS scripts from HTML source (firefox-devtoo…
6c5d43b 
…ls#4684)"

This reverts commit b98fa76.
@codehag

codehag commented Dec 4, 2017

Copy link
Copy Markdown
Contributor

this change needed to be backed out (#4840), it was causing a memory leak on linux machines. it looks like it was coming from an imported dependancy, can you take a look @ryanjduffy ?

codehag added a commit to codehag/debugger.html that referenced this pull request Dec 4, 2017
@codehag
Revert "Fix including non-JS scripts from HTML source (firefox-devtoo…
8a3a4de 
…ls#4684)"

This reverts commit b98fa76.
codehag added a commit that referenced this pull request Dec 4, 2017
@codehag
Release 11 22 (#4840)
19585cb 
* Update Release 11-22

* Revert "Fix including non-JS scripts from HTML source (#4684)"

This reverts commit b98fa76.

* new manifest
@ryanjduffy

ryanjduffy commented Dec 5, 2017

Copy link
Copy Markdown
Contributor Author

Will do! I've started on a smaller, special-purposed sub-module to replace the imported dependency which should both address the leak (assuming I code it correctly) and the file size concern @jasonLaster referenced in ryanjduffy/parse-script-tags#6.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ryanjduffy @jasonLaster @codehag