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

Fix including non-JS scripts from HTML source #4684

Merged
merged 1 commit into from
Nov 14, 2017

Conversation

ryanjduffy
Copy link
Contributor

@ryanjduffy ryanjduffy commented Nov 13, 2017

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>

@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
Copy link
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 added a commit to codehag/debugger.html that referenced this pull request Dec 1, 2017
@codehag
Copy link
Contributor

codehag commented Dec 4, 2017

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 added a commit that referenced this pull request Dec 4, 2017
* Update Release 11-22

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

This reverts commit b98fa76.

* new manifest
@ryanjduffy
Copy link
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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants