Closed Bug 956567 Opened 6 years ago Closed 6 years ago

[prettify] Pretty print in the debugger produces invalid js

Categories

(DevTools :: Debugger, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 30

People

(Reporter: vporof, Assigned: brion)

References

Details

Attachments

(1 file, 1 obsolete file)

STR:

1. Go to http://todomvc.com/labs/dependency-examples/enyo_backbone/
2. Open debugger
3. Make sure app.js is selected and pretty printed

Line 4144 looks like this:

h.innerHTML = '  <link/><table></table><a href='/a'>a</a><input type='checkbox'/>',

Let me break that down for you:

foo.bar = "str" /a "str" checkbox "str"

This makes Parser.jsm sad when searching for functions and doing other smartsy stuff. It's also invalid javascript, but that's a technicality.
(In reply to Victor Porof [:vp] from comment #0)
> It's also invalid javascript, but that's a technicality.

lol
Priority: -- → P3
Summary: Pretty print in the debugger produces invalid js → [prettify] Pretty print in the debugger produces invalid js
This looks like the same as bug 964103.
Duplicate of this bug: 964103
Patch to escape single quotes in strings in the debugger's pretty printer. Resolves round-tripping issues with strings like 'Can\'t' or "Can't"; both now turn into correct 'Can'\t' instead of broken 'Can't'.
Comment on attachment 8374048 [details] [diff] [review]
prettyprint-singlequote-escape.patch

Please feel free to bump review around, I'm a bit unclear on the protocol here. :) Thanks!
Attachment #8374048 - Flags: review?(nfitzgerald)
And here's a pull req on the separate pretty-fast repo on github: https://github.com/mozilla/pretty-fast/pull/3
Copying comment from github for posterity:

"""
Thanks for taking this on Brion.

1. Tests are failing. At a glance it looks like pretty fast is giving the correct output but the test needs to be updated. Make sure that the tests are passing (via npm test on the command line) before updating this pull request.

2. We need a test that specifically tests that we escape single quotes properly. I think actually updating the test failing above would be perfect. Embarrassingly enough, I think that test is asserting the incorrect behavior that your patch is fixing! Woops!
"""
Assignee: nobody → brion
Status: NEW → ASSIGNED
Attachment #8374048 - Flags: review?(nfitzgerald)
Updated patch which also updates the test case for the fix.

Added a second commit to pull req version with this fix as well: https://github.com/mozilla/pretty-fast/pull/3
Attachment #8374801 - Flags: review?(nfitzgerald)
Comment on attachment 8374801 [details] [diff] [review]
prettyprint-singlequote-escape2.patch

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

Thanks!
Attachment #8374801 - Flags: review?(nfitzgerald) → review+
Attachment #8374048 - Attachment is obsolete: true
https://hg.mozilla.org/integration/fx-team/rev/b7240f73df3f

Brion, please make sure that future patches follow the guidelines below. Makes life easier for those landing on your behalf :)
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Thanks! *bookmarked & .hgrc edited*
https://hg.mozilla.org/mozilla-central/rev/b7240f73df3f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 30
Duplicate of this bug: 952864
Status: RESOLVED → VERIFIED
Duplicate of this bug: 975464
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.